On Fri, Aug 27, 2010 at 05:56:42PM +0100, Toby Gray wrote:
> Hi,
>
> I've updated my branch at git://github.com/tobygray/barry-usbrelay.git
> with all the review comments and suggestions (currently
> 7a1919fb7011d15dac3f). Thank you for all the feedback, it's certainly
> lead to a simpler and easier to use API. I've added documentation and an
> example which talks to the 'USB Demo' supplied with RIM code.
>
> Does anyone have any further suggestions or comments on the API changes
> I've made? It'd also be great if someone other than me could go through
> the 'how do I' documentation to make sure I've not missed any vital
> steps from it!
Hi Toby,
Thanks again very much for your work on this. I have rebased your branch
on to the latest master and pushed it out to the repos with some changes
of my own. I rebase to make it easier for me to keep the CVS repositories
on sourceforge up to date. Fortunately, there were no conflicts when
I rebased your work.
I do have some questions and issues that need to be addressed before
releasing 0.17, though.
+void RawChannel::OnOpen()
+{
+ // Enable sequence packets so that DataSendAck callback and close can be
+ // implemented
+ m_zero_registered = true;
+ m_socket->HideSequencePacket(false);
+ std::tr1::shared_ptr<SocketRoutingQueue::SocketDataHandler> callback;
+ callback.reset(new RawChannelSocketHandler(*this));
+ m_con.m_queue->RegisterInterest(0, callback);
+ // Get socket data packets routed to this class as well if using
callback
+ // otherside just request interest
+ if( !m_callback ) {
+ // Don't want to be called back immediately on data
+ callback.reset();
+ }
+ m_socket->RegisterInterest(callback);
+}
Why are you registering an interest with a potentially empty callback?
Are you trying to throw away data? Or are you assuming the socket class
will see the empty callback and ignore registering it?
--- a/src/socket.cc
+++ b/src/socket.cc
@@ -501,6 +501,12 @@ SocketHandle SocketZero::Open(uint16_t socket, const char *
password)
// fall through to success check...
}
+ if( packet.Command() == SB_COMMAND_CLOSE_SOCKET )
+ {
+ eout("Packet:\n" << receive);
+ throw Error("Socket: Socket closed when trying to open");
+ }
+
if( packet.Command() != SB_COMMAND_OPENED_SOCKET ||
packet.SocketResponse() != socket ||
packet.SocketSequence() != closeFlag )
@@ -566,9 +572,15 @@ void SocketZero::Close(Socket &socket)
{
// reset so this won't be called again
socket.ForceClosed();
-
- eout("Packet:\n" << response);
- throw BadPacket(rpack->command, "Socket: Bad CLOSED packet in
Close");
+
+ if( rpack->command == SB_COMMAND_REMOTE_CLOSE_SOCKET ) {
+ eout("Remote end closed connection");
+ throw BadPacket(rpack->command, "Socket: Remote close
packet in Close");
+ }
+ else {
+ eout("Packet:\n" << response);
+ throw BadPacket(rpack->command, "Socket: Bad CLOSED
packet in Close");
+ }
}
Can you explain the logic here? How can CLOSE_SOCKET be returned when
we just sent the password handshake? And are all these errors worthy
of exceptions?
I assume this code has something to do with the new raw channel protocol,
and I just want to better understand it.
In m_raw_channel.cc:
+#define RAW_HEADER_SIZE 4
That's a bug... this should go in src/protostructs.h as a struct.
You don't want to rely on MIN_PACKET_DATA_SIZE to always be correct in
src/protostructs.h (see m_raw_channel.cc line 164) when there are
header size constants elsewhere in the code. Header sizes and definitions
need to be defined in protostructs.h and used via #define or sizeof()
elsewhere in the code.
If you find places where I've also violated this rule, please let me know,
since that's a bug too. :-) It's best to keep low level protocol information
in one place.
Actually, in RawChannel::Send(), you simply use struct Packet, filling
in the first 4 bytes with socket and size information. If this is all
that RAW_HEADER_SIZE represents, then you can remove it, and use
SB_JVMPACKET_HEADER_SIZE, or you can create your own SB_RAWPACKET_HEADER_SIZE
along with a struct for it, similar to struct JVMPacket and struct Packet.
Similarly, RawChannel::MaximumSendSize() should just return a #define
from protostructs.h. The low level packet size and structure information
is stored in that header as much as possible.
In router.h:
There's a new flag, m_seen_usb_error, and it needed a way to reset it,
and be an easy way to deal with USB errors.
I figured that the best place to recover from USB errors is in the
application code that calls Router::SetUsbDevice(), so I added a
simple callback there, and a call to clear the flag in case of
recovery.
- Chris
------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
