Some small fixes and improvements extracted from PR1238 #1565
Conversation
STATE_TX_WAIT is meant to be a single bit but is defined as two bits, therefore overlapping with STATE_RX. This causes incorrect results when the code uses the bits to handle the state machine. Fix this. Signed-off-by: Frieder Schrempf <frieder@fris.de>
There is no reason to not use the reset pin as the RAK4630/31 module has it connected internally. Signed-off-by: Frieder Schrempf <frieder@fris.de>
If the radio driver state machine is not in receive mode, this means that processing of a packet is still in progress and we are not in an idle state. Signed-off-by: Frieder Schrempf <frieder@fris.de>
This makes the code easier to read and allows for easier changing of the hardcoded values. Signed-off-by: Frieder Schrempf <frieder@fris.de>
When a CLI command is issued through the serial interface, extend the timeout for going to sleep to give the user more time for issuing more commands. Signed-off-by: Frieder Schrempf <frieder@fris.de>
| #define STATE_IDLE 0 | ||
| #define STATE_RX 1 | ||
| #define STATE_TX_WAIT 3 | ||
| #define STATE_TX_WAIT 2 |
There was a problem hiding this comment.
I don't follow what this is supposed to fix?
There was a problem hiding this comment.
Did you see the commit message?
STATE_TX_WAITis meant to be a single bit but is defined as two bits,
therefore overlapping withSTATE_RX. This causes incorrect results
when the code uses the bits to handle the state machine.
Unless I'm getting it all wrong (which totally could be the case) you meant to use the STATE_* defines as single-bit masks for the static volatile uint8_t state variable. But 3 is not a single bit but two bits. Therefore currently if state is set to STATE_TX_WAIT, then state & STATE_RX will return true, which seems unexpected and looks wrong to me.
I don't now if this has any negative impact with the current implementation, but it certainly caused problems with the code I was working at here: a6ce03d.
And anyway, it seems like a good idea to fix this no matter if it is currently causing any issues or not as it has the potential to cause problems in the future.
There was a problem hiding this comment.
It looks like there is no actual impact at the moment, as checks for STATE_RX are currently all done with == to check for STATE_RX exclusively.
2 participants
While working on #1238 I discovered a bug and made a few small improvements which can be reviewed and merged separately.
RadioLibWrappers.cppstate machinehasPendingWork()which is used to decide when the system is idle and can enter powersaving modeconstexprvalues for powersaving timings instead of inline literals