I think the wrong constant was used, but also I haven't done a PR as I don’t think this is necessary?
1) CustomLR2021.h
It is harmless but the parameter passed to startReceive is the module's constant (RADIOLIB_LR2021_IRQ_PREAMBLE_DETECTED), it should be the generic RadioLib one (RADIOLIB_IRQ_PREAMBLE_DETECTED).
int16_t startReceive() override {
// include the PREAMBLE_DETECTED irq bit in reported flags
return LR2021::startReceive(RADIOLIB_LR2021_RX_TIMEOUT_INF, RADIOLIB_IRQ_RX_DEFAULT_FLAGS | (1UL << RADIOLIB_LR2021_IRQ_PREAMBLE_DETECTED), RADIOLIB_IRQ_RX_DEFAULT_MASK, 0);
}
I dont think it has an effect as it expands to (1 << (1 << 5)) or 1 << 32, which (depending on the processor) should resolve to 0 (no effect) or 1 (RXDONE which is already set).
2) Is this necessary?
I could be reading things wrong but the comment seems to be incorrect? I don’t think this changes any behaviour.
RadioLib doesn’t modify the IRQ flags returned – what this controls is which IRQ flags get mapped to the DIO pin and trigger setPacketReceivedAction (in MC: RadioLibWrapper SetFlag).
Here we’re adding the preamble detect to the flags but not to the mask. This ultimately doesn’t seem to change anything:
- For the LR11/LR21/SX127x chips these two parameters are bitwise-& together so the effect disappears
- For the SX126x/SX128x family this sets the preamble as something that can trigger the DIO pin but doesn’t set the pin to toggle on it.
The fix for this isn't to add this to the mask - we'll just be triggering SetFlag on preamble and RX_DONE and will block the processor from seeing any further interrupts until the preamble is cleared (which clears the DIO pin)(while true the other IRQ is RX_DONE so unless preamble timeout is longer than the smallest packet this is OK). IsReceiving() seems to get called frequently enough that just detecting timing from the IRQ state is enough, so if I'm correct we should just pull this override?
Am I missing something here?
I think the wrong constant was used, but also I haven't done a PR as I don’t think this is necessary?
1) CustomLR2021.h
It is harmless but the parameter passed to startReceive is the module's constant (
RADIOLIB_LR2021_IRQ_PREAMBLE_DETECTED), it should be the generic RadioLib one (RADIOLIB_IRQ_PREAMBLE_DETECTED).I dont think it has an effect as it expands to (1 << (1 << 5)) or 1 << 32, which (depending on the processor) should resolve to 0 (no effect) or 1 (RXDONE which is already set).
2) Is this necessary?
I could be reading things wrong but the comment seems to be incorrect? I don’t think this changes any behaviour.
RadioLib doesn’t modify the IRQ flags returned – what this controls is which IRQ flags get mapped to the DIO pin and trigger setPacketReceivedAction (in MC: RadioLibWrapper SetFlag).
Here we’re adding the preamble detect to the flags but not to the mask. This ultimately doesn’t seem to change anything:
The fix for this isn't to add this to the mask - we'll just be triggering SetFlag on preamble and RX_DONE
and will block the processor from seeing any further interrupts until the preamble is cleared (which clears the DIO pin)(while true the other IRQ is RX_DONE so unless preamble timeout is longer than the smallest packet this is OK). IsReceiving() seems to get called frequently enough that just detecting timing from the IRQ state is enough, so if I'm correct we should just pull this override?Am I missing something here?