openenergymonitor / emonpi

Raspberry Pi Based Energy Monitor. Hardware, Firmware & related software for the PI.
https://guide.openenergymonitor.org/setup
270 stars 113 forks source link

"keep alive" too aggressive? #115

Closed pb66 closed 4 years ago

pb66 commented 5 years ago

https://github.com/openenergymonitor/emonpi/blob/2.9.1/firmware/src/src.ino#L269-L271

if ((now - last_rf_rest) > RF_RESET_PERIOD) {
      rf12_initialize(nodeID, RF_freq, networkGroup);                             // Periodically reset RFM69CW to keep it alive :-(
    }

This code appears to run every iteration of the main loop after the first 60s because last_rf_rest is declared as 0 during set up and never gets updated.

glynhudson commented 5 years ago

Good spot. Have you heard if this is this causing any known issues?

Maybe it's not worth changing since it's proven stable in its current form. The RF on my early test emonPi has not had a single dropout in over 4yrs of operation.

pb66 commented 4 years ago

TBH I wasn't aware of any actual reported cases but there is now that someone has tried recompiling the emonpi FW with some changes. See https://community.openenergymonitor.org/t/emonpi-firmware-build/12428/37?u=pb66

adtyne commented 4 years ago

This issue gave me serious problems building a working customised firmware in recent days, adding a single line to update last_rf_rest fixed my issues immediately. I'm not sure why this hasn't caused a problem in your build environment, but I hit problems using PlatformIO on my Pi, PlatformIO on my desktop machine and using the Arduino IDE and resolving the dependencies manually.

Whilst this might not be causing problems in the official build, and may only cause problems for people wanting to do extend the functionality to suit their own purposes. I don't see that applying this fix would be a determent to the wider user base, and it might just prevent a problem down the line.

glynhudson commented 4 years ago

Fixed by https://github.com/openenergymonitor/emonpi/commit/e3ca73b876ebac2c324eeba84f8d517dc34e543d

Please let me know if V2.9.2 release works for you: https://github.com/openenergymonitor/emonpi/releases/tag/2.9.2

adtyne commented 4 years ago

Hello Glyn

I’ve been running that firmware on my system since you published the release without any problems.

Many Thanks

Alistair Tyne

From: Glyn Hudson notifications@github.com Sent: 17 November 2019 14:10 To: openenergymonitor/emonpi emonpi@noreply.github.com Cc: Alistair Tyne alistair@sy23.co.uk; Manual manual@noreply.github.com Subject: Re: [openenergymonitor/emonpi] "keep alive" too aggressive? (#115)

Fixed by e3ca73bhttps://github.com/openenergymonitor/emonpi/commit/e3ca73b876ebac2c324eeba84f8d517dc34e543d

Please let me know if V2.9.2 release works for you: https://github.com/openenergymonitor/emonpi/releases/tag/2.9.2

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/openenergymonitor/emonpi/issues/115?email_source=notifications&email_token=AKEF5MVVC3HFO255EXXKRXDQUFGDVA5CNFSM4ISWQMG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEIM32Y#issuecomment-554749419, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKEF5MQ5LKPK5NLD4PXQX6LQUFGDVANCNFSM4ISWQMGQ.

glynhudson commented 4 years ago

Thanks, the change has been incorporated into a stable firmware release. Our internal testing has also not highlighted any issues. Thanks for your help.