FaradayRF / Faraday-Software

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

Proxy Incorrectly Handling Opened RX Port With No New Data #162

Open kb1lqd opened 7 years ago

kb1lqd commented 7 years ago

Summary

Recent proxy.py updates incorrectly handle an open RX port that has no new data as it returns False when no data is to be returned.

Problem Explanation

False is returned when no data in available from an open port and proxy uses this as normal data. Telemetry and other single data item programs just ignore but multi-packet protocols such as Hermes messenger hang and fail to receive packets.

Environment

Software

Master software

Hardware

REV D1

Supporting Information

The image below shows test code additions that fix the problem by checking for False and disregarding the returned item as data.

image

kb1lqc commented 7 years ago

Slightly confused on this ticket, what should Proxy be doing?

kb1lqd commented 7 years ago

I placed a print statement in the hermes message script state machine for RX to print all fragment frames being parsed on a new reception. I only see the start packet arrive...

image

Printing out the TX path as the packets are created shows packets are being created and the START, DATA, STOP frame identifiers are correct:

image

image

Something is clearly not letting RX retrieve all the packets. Is it the updates for the CPU reduction? https://github.com/FaradayRF/Faraday-Software/pull/157

kb1lqd commented 7 years ago

I placed a print showing the count of port 3 items in proxy when sending a message through Hermes. the correct count of 3 was observed: image

image

But the Hermes receiver got nothing (unedited): image

Printing queue size in Hermes RX queuelen function shows only occasionally a single frame gets retrieved...

image

kb1lqd commented 7 years ago

Printing GetDict[] dictionary keys in proxy is showing that port 3 is not appending dictionary items!

image

image

kb1lqd commented 7 years ago

Using the proxy log functionality I can see 3 packets getting into proxy:

image

They are decoding correctly manually:

image

Oh damn.... @kb1lqc @reillyeon is this a bug due to the global getDicts but now there are multiple threads reading a single global item after I updated Proxy to split up each Faraday unit into it's own thread?

kb1lqc commented 7 years ago

@kb1lqd well this almost sounds like nominal Proxy operation. As designed it is only intended to store a queue and when anything reads the data it will pop it off. So if you append to a queue and some other thread pops it off then that is technically nominal.

Is there really a second thread reading from port 3?

kb1lqd commented 7 years ago

I'm saying the data is not going into the queue. The queue for port 3 gets removed or deleted... data comes into proxy but gets lost trying to append to the getdict queue for port 3.

On Thu, Apr 20, 2017 at 12:59 AM Bryce Salmi notifications@github.com wrote:

@kb1lqd https://github.com/kb1lqd well this almost sounds like nominal Proxy operation. As designed it is only intended to store a queue and when anything reads the data it will pop it off. So if you append to a queue and some other thread pops it off then that is technically nominal.

Is there really a second thread reading from port 3?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/FaradayRF/Faraday-Software/issues/162#issuecomment-295619572, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUsyNONEjdGI0EidQ7cdJ_8QLaEA2JJks5rxxBTgaJpZM4M-vad .

kb1lqc commented 7 years ago

This is interesting sing deque is supposed to be thread-safe:

https://docs.python.org/2/library/collections.html#deque-objects

A good example is "burning a candle from both ends": https://pymotw.com/2/collections/deque.html#consuming

kb1lqd commented 7 years ago

We are storing the dequeue's in a dictionary... not thread safe.

On Thu, Apr 20, 2017 at 11:45 PM Bryce Salmi notifications@github.com wrote:

This is interesting sing dequeu is supposed to be thread-safe:

https://docs.python.org/2/library/collections.html#deque-objects

A good example is "burning a candle from both ends": https://pymotw.com/2/collections/deque.html#consuming

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/FaradayRF/Faraday-Software/issues/162#issuecomment-296095243, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUsyNJKI-ZB436jZHXMgIrPIkU0Yh-9ks5ryFB8gaJpZM4M-vad .

kb1lqc commented 7 years ago

OK yeah that is correct and obvious in retrospect... Stackexchange explains it well: https://stackoverflow.com/questions/3358770/python-dictionary-is-thread-safe

Looks like we will need a new approach to this. Any way we can quickly test out the theory with minimal impact? Maybe hack out the dictionaries so only dequeus are used and assume only one station ever?