mysensors / NodeManager

Plugin for a rapid development of battery-powered sensors
130 stars 82 forks source link

SensorSignal reports 0 if MY_SIGNAL_REPORT_ENABLED not defined in sketch #504

Closed liwenyip closed 4 years ago

liwenyip commented 4 years ago

Hi,

Thank you @user2684 for creating and maintaining this project, it is AWESOME

Nodemanager v1.8 MySensors v2.3.2 Arduino Pro Mini 3.3V

I've noticed that you have to #define MY_SIGNAL_REPORT_ENABLED at the top of the sketch, otherwise SensorSignal always reports zero.

MY_SIGNAL_REPORT_ENABLED is defined at the top of NodeManager/sensors/SensorSignal.h, but that's too late - it has to be defined before thetransportGetSignalReport function (MySensors/core/MyTransport.cpp:1092) is compiled, otherwise that function is compiled to always return zero.

Looking for your thoughts on solution before I submit a PR:

  1. Put a comment in the template sketch:

    //#include <sensors/SensorSignal.h>
    //SensorSignal signal;
    // NB uncomment #define MY_SIGNAL_REPORT_ENABLED above for this to work
  2. Instead of defining MY_SIGNAL_REPORT_ENABLED in SensorSignal.h, throw a compiler error if it has not already been defined:

    #ifndef MY_SIGNAL_REPORT_ENABLED
    #error "MY_SIGNAL_REPORT_ENABLED must be defined in the sketch, before #include <MySensors_NodeManager.h>"
    #endif
  3. Any other more elegant ideas?

Thanks in advance

user2684 commented 4 years ago

Thanks for reporting it, never noticed this thing. Wonder why all my sensors are correctly reporting the signal without any change. Do you get a 0 signal reported instead? If confirmed, I'd go for option 2, seems to me the most clean approach. Thanks!

user2684 commented 4 years ago

Hi, I was finally able to reproduce the issue, seems related to the latest version of MySensors library this is why was not showing up straight away. Fixed through #517 . Thanks!