Skip to content

fix race condition with receiving messages - #36

Open
neilgupta wants to merge 1 commit into
etrombly:masterfrom
Hiome:raceCondition
Open

fix race condition with receiving messages#36
neilgupta wants to merge 1 commit into
etrombly:masterfrom
Hiome:raceCondition

Conversation

@neilgupta

Copy link
Copy Markdown

We no longer call receiveBegin() from inside receiveDone() if no data's
ready. Previously, if interruptHandler triggerred while receiveDone()
was processing, receiveBegin() would wipe out the received data. This
could be mitigated with sleeps between loops to decrease the odds of
such a scenario, but it's far more reliable to just let the user
explicitly call receiveBegin() once they have processed the latest
message. This should fix issue #29

We no longer call receiveBegin() from inside receiveDone() if no data's
ready. Previously, if interruptHandler triggerred while receiveDone()
was processing, receiveBegin() would wipe out the received data. This
could be mitigated with sleeps between loops to decrease the odds of
such a scenario, but it's far more reliable to just let the user
explicitly call receiveBegin() once they have processed the latest
message. This should fix issue etrombly#29

@electronicsuk1 electronicsuk1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you verified that this works? My transmitter node is an Arduino using LowPowerLab's RFM69 library (latest commit 855889c from March 3rd) and receiver node is an RPi. I've tried your commit and (for me at least) the Arduino transmitter node still doesn't see ACK for ~90% of packets, even though the RPi is definitely receiving them. Transmitting from the RPi to the Arduino works fine. I know it's an issue at the Pi end as I've tried the same radios attached to an Arduino at both ends and it works fine.

@etrombly

Copy link
Copy Markdown
Owner

I'll have to set back up a pi to test with. Don't have anything setup right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants