RFM69::initialize(...) doesn't always set the radio to the same state

[jgillula]

Say a radio is in listen mode, and then a new sketch that doesn't use listen mode is uploaded to the Moteino (e.g. via the Arduino IDE). When the new sketch runs RFM69::initialize(...) the radio stays in listen mode since nothing has told it to turn listen mode off. Only resetting the power will turn of listen mode, or making sure to always call RFM69::listenModeEnd() right after RFM69::initialize(...). The former is annoying, and the latter would force anyone to define RF69_LISTENMODE_ENABLE even if they're not using listen mode.

Instead, RFM69::initialize(...) should probably turn off listen mode if it's on, regardless of whether or not RF69_LISTENMODE_ENABLE is defined, so that calls to RFM69::initialize(...) always put the radio into the same starting state.

PR #171 fixes this by moving some of the code in RFM69::listenModeEnd() to RFM69::initialize().

[LowPowerLab]

I appreciate the effort but there are two issues: 1) ListenMode was always experiemental and always excluded by default via a #define. Adding code/size to all compiled code, to handle ListenMode in initialize(), that is by default excluded via the #define, and this just in case someone may program a sketch then they program another without power cycling, makes little sense. If you know you're using ListenMode, then there is code to handle that case and you should use it. We do not want to increase compile size in perpetuity for such edge cases. If that were the case, then to be inclusive we would need to completely reset all registers in the radio not just listenMode, because someone is using some non default radio mode. We either do it 100% or not at all, everything in between is a code smell if we want to be true to best practices.

2) Separation of concerns. I see ListenMode almost like a separate library. The real issue here is that ListenMode is deprecated and I will probably remove it from the source code entirely, since it is was determined to be unreliable and this is documented in the forum. Bonus) There is a RESET jumper on some of the Moteino boards, or it can be soldered easily to a pin of choice. That can be toggled to ensure a proper POR register defaults. Also, I suggest a RESET jumper on any custom designs for this very purpose.

[jgillula]

Makes sense!

For (1), I would actually lean toward resetting all the registers, and I'd be happy to code up that PR if you'll accept it. If not, no worries--I understand.

For (2), I haven't encountered the unreliability described in the forum, but maybe I've been lucky. Since I rely on ListenMode for my projects, I'd be willing to do the work to break ListenMode out into a separate library that depends on the RFM69 library if you want. If I did that, would you want it to be in the RFM69 library (like the RFM69_ATC class)? Or a whole separate Github repo/library in the Arduino library manager?

And thanks for the tips about the bonus! I should probably hook up the reset jumper--it just hadn't occurred to me. X-)

[LowPowerLab]

Thanks for the suggestions. To 1): I imagine you're thinking to expand the register set in initialize() with all registers (except perhaps some that are never touched). I am worried about the sketch impact this will have on all compiled code, I have some 328p code that is near the limit and we're looking for every way to optimize and not increase compiled size since that firmware is also OTA updated. I wish there was a software reset in the radio, but sadly there is none, just the RST pin (active high, 100us).

2) I was looking for an opportunity to separate ListenMode from the main RFM69 class. As a side note - I was also looking for an opportunity to merge the RFM69_ATC into the main RFM69 class - this is highly valuable and proven to help with reducing premature battery wear. I think a separate RFM69_ListenMode.h would be appropriate. Could follow the ATC pattern. But then I'm not sure if it's appropriate to inherit from ATC - my feeling is that it's not, and should just be another polymorphic subclass of RFM69.

[jgillula]

1) Makes sense!

2) Also makes sense--I'll work on a PR that breaks out listen mode into RFM69_ListenMode.h and RFM69_ListenMode.cpp as a polymorphic subclass of RFM69.