FaradayRF / Faraday-Software

Faraday node software
https://www.faradayrf.com
Other
48 stars 19 forks source link

Proxy Data Socket #251

Closed kb1lqc closed 7 years ago

kb1lqc commented 7 years ago

This branch is experimental and adds a socket server to Proxy which provides TX and RX capabilities for a Faraday radio. It also handles multiple radios and will automatically assign appropriate ports albeit a bit hardcoded for now. We should look at this PR as more of a bleeding edge rather than polished product. I'll be adding documentation for it shortly in a follow up commit/PR.

Changelog

Connecting to the port 10000 with a client and sending data will result in Faraday transmitting the data. Any received data on the same Faraday will show up on port 10010 and a call with recv() after sending a single byte of data (weird) will result in the received data streaming to the socket 10010. Two Faraday radios are supported and may communicate with each other (broadcast) while connected to a multi-unit configured Proxy application.

@jbemenheiser @reillyeon @el-iso

kb1lqd commented 7 years ago

Issue 1

If I connect a client and don't send data but close the client then proxy crashes. The client program opens a socket with proxy but waits for user input before sending. I don't send data and just close:

image

Closing the client without sending causes the 'data' references before assignment crash.

image

kb1lqd commented 7 years ago

Issue 2

Closing a connection after sending any data causes Proxy to continuously transmit (I assume the last item in the buffer):

Proxy displays:

image

I must close proxy to stop transmitting.

kb1lqc commented 7 years ago

I can't reproduce your error when not sending data. Pretty sure https://github.com/FaradayRF/Faraday-Software/pull/251/commits/ccedebad374ae6ea777c49af42b202c4855adb6d fixed both your stated issues.

kb1lqd commented 7 years ago

Issue 1 fixed confirmed.

Issue 2 cannot reproduce either. I was due to an older installation being present. Disregard.

kb1lqd commented 7 years ago

Issue 3

This program should return the exact bytestream assumption a socket implies. The program data should include in EVERY proxy packet layer 4 a simple packet that has payload length so that the exact payload and only that payload can be returned.

Example: Transmitting a small amount of data like "Test" should only return "Test" and not followed by the remaining padding.

image

Should NOT return the padding:

image

kb1lqd commented 7 years ago

Confirmed Issue 3 resolved:

image

image

The receiver socket worker was updated to return only the data that was placed into the socket. Fragmentation is had for free due to the queue functionality.

kb1lqd commented 7 years ago

Issue 4 (@KB1LQC)

When testing the transmission and reception I found that the receiver client when quitting occasionally caused this error in proxy that prevented re connection (receiver only) until proxy was restarted.

image

kb1lqc commented 7 years ago

Looking into why we drop packets @kb1lqd I noticed the following:

2017-08-01 22:10:27,651 - Proxy - INFO - 123
2017-08-01 22:10:27,651 - Proxy - INFO - '\x00\x00\x1d24, 25, 26, 27, 28, 29, 30, 3 16, 17, 1\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff'
2017-08-01 22:10:27,703 - Proxy - INFO - 42
2017-08-01 22:10:27,703 - Proxy - INFO - '\x00\x00#1, 32, 33, 34, 35, 36, 37, 38, 39, 7, 1'
2017-08-01 22:10:27,707 - Proxy - WARNING - unpack requires a string argument of length 123
2017-08-01 22:10:27,709 - Proxy - WARNING - 42
2017-08-01 22:10:27,709 - Proxy - WARNING - '\x00\x00#1, 32, 33, 34, 35, 36, 37, 38, 39, 7, 1'
2017-08-01 22:10:27,766 - Proxy - INFO - 123
2017-08-01 22:10:27,766 - Proxy - INFO - '\x00\x00"40, 41, 42, 43, 44, 45, 46, 47, 48\x00\xc0\x1e\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff'

The above output is from always printing the length of the data to be unpacked as well as the data itself, the warning output is the length and data when struct.unpack fails.

Do you find it odd that we get length 42 data which will obviously fail a struct.unpack that is expected 123 data of which 121 bytes is the payload? Is your uart layer doing some of the fragmentation with RF? Should we be looking for length 42 and acting accordingly?

Edit:

BOOM look at that, it must be. The missing/dropped line is perfectly equal to the missing data between the RX test client received output...

image

kb1lqc commented 7 years ago

OK @kb1lqd with https://github.com/FaradayRF/Faraday-Software/pull/251/commits/c1d692fa7cf3bd55dc4c1717ce6a3ae1afac4dba I appear to be getting all data sent over the original link. Below is a valid count between 0 and 299 in my test string of a list counting all 300 numbers:

image

Not sure if this is what you intended but it appears to work. In the end I think the uart code needs to be shored up but this seems to work for now.

kb1lqd commented 7 years ago

@kb1lqc in regards to https://github.com/FaradayRF/Faraday-Software/pull/251#issuecomment-319569549

Yes, this is only done on the MSP430 firmware where the link between UART -> RF and RF-> UART occurs. The 128 byte packet is split into 3 RF packets and reassembled on the other side. The result is through UART the same item appears out of the RX Faraday unit. You should not see this in the software... Maybe something is getting through the logic...

kb1lqd commented 7 years ago

@kb1lqc Seems too work better and not error out but I'm seeing issues with closing the RX client and then trying too reconnect... The RX program just stays blank... No apparent error though.

image

kb1lqc commented 7 years ago

@kb1lqd it's possible I need to be tricky with my while loops. I apparently achieve it in this commit earlier. However, the problem seems to be arising from the fact that I need getDicts[unit][1] to exist but until data is RX'd it doesn't exist. When I apparently try to mitigate that I also am missing something that exits the thread so reconnection isn't possible. This is tricky...

kb1lqd commented 7 years ago

OK @kb1lqc is this something you think you can achieve again? I think it is pretty critical considering every reconnect would need a restart of proxy. We're very close to closing this initial PR otherwise.

reillyeon commented 7 years ago

It would be nice if this used a WebSocket instead of a regular TCP/IP socket so that it integrated into the existing REST API. Not something to be fixed in this PR but something to think about when we switch to a framework that can support WebSockets.

kb1lqc commented 7 years ago

@reillyeon We locked at websockets but it appeared they were not backwards compatible with normal socket use.

kb1lqd commented 7 years ago

@kb1lqc Do you think you can try to get the RX no-reconnect socket fixed? I may need to pre-approve before this weekend. Not a huge deal but certainly is an annoyance.

kb1lqc commented 7 years ago

@kb1lqd Yeah that's my main goal right now. Pesky. Will continue it in the morning.

kb1lqc commented 7 years ago

Quick look at it this morning and I notice the following:

I'm struggling for time lately so hopefully I can get this done over the weekend. Did you want to approve under certain conditions be met? @kb1lqd? I can also just push it when ready with admin privileges though I am not the biggest fan of those.

kb1lqc commented 7 years ago

Alright, as of https://github.com/FaradayRF/Faraday-Software/pull/251/commits/d5e5c30604ae5c29ea37216463d7669592e964aa I believe I fixed the client-side reconnect issues as well as cleaned up the code. Is it stellar code? No, but it's good enough right now.

Will perform some manual packaging tests and such before pulling this in per @kb1lqd verbal approval already obtained (he's on vacation) contingent on the client reconnection issue being mitigated.

@reillyeon @el-iso

kb1lqd commented 7 years ago

Awesome! I have no computer do you're on your own. Good luck, we're all counting on you.

On Aug 6, 2017 8:55 PM, "Bryce Salmi" notifications@github.com wrote:

Alright, as of d5e5c30 https://github.com/FaradayRF/Faraday-Software/commit/d5e5c30604ae5c29ea37216463d7669592e964aa I believe I fixed the client-side reconnect issues as well as cleaned up the code. Is it stellar code? No, but it's good enough right now.

Will perform some manual packaging tests and such before pulling this in per @kb1lqd https://github.com/kb1lqd verbal approval already obtained (he's on vacation) contingent on the client reconnection issue being mitigated.

@reillyeon https://github.com/reillyeon @el-iso https://github.com/el-iso

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/FaradayRF/Faraday-Software/pull/251#issuecomment-320562973, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUsyMEPxoXSx0kfE69bgN7kxUbIG3b1ks5sVoqlgaJpZM4OpM7b .

kb1lqc commented 7 years ago

Tested by hand and appears to work as expected:

Please note:

Will use my admin privileges to merge.

Thanks @kb1lqd!

@reillyeon @el-iso