NodeManager icon indicating copy to clipboard operation
NodeManager copied to clipboard

Sensors do not report values if interrupts is enabled

Open j54n1n opened this issue 2 years ago • 2 comments

Hello,

I think I found a logical error that inhibits reporting of sensor values when interrupts are enabled and a sensor with high interrupt frequency is used.

In my case it was a weather station node with some non-interrupt sensors (i.e. reporting at a given time interval) and a wind speed sensor using SensorPulseMeter with smart sleep disabled and using an RTC clock module. The wind speed was reported always, but the other sensors would report only when the wind speed was very low.

The Anemometer from Davis DW-6410 that I am using is specified to measure wind speeds up to 322km/h which translates into an interrupt frequency of about 89Hz. But nevertheless even at lower speeds greater than 1Hz I was already getting no reports from the other sensors.

So I looked at the code from Node.cpp and I found that at line 291 the else if block allows only reporting of the other sensors if no _last_interrupt_pin was recognized. At very low speeds (i.e. less than 1Hz interrupt frequency) there is at least some chance that one of the loop cycles goes through without the interrupt pin assigned, but at higher speeds this will not occur since interrupt pin is assigned ever so more.

I think the reporting within the loop function can be arranged in the same way as before but instead of using the else if block a else block should be enough to allow both type of sensors to be reported. At least in my local version where I tested this change it seems to be working also at high interrupt frequencies. I will propose a PR that fixes this issue.

j54n1n avatar Aug 22 '21 08:08 j54n1n

Thanks good finding! I do remember though once upon a time that "else if" was added for a reason something like there were situations where interrupt code was executed unintentionally multiple times because the variable was not reset in between. But I cannot remember which was really the case so I'd rather trust your tests. Just a question before merging: is this supposed to be working fine for both non sleeping and sleeping nodes right? Thanks!

user2684 avatar Aug 28 '21 09:08 user2684

Thanks good finding! I do remember though once upon a time that "else if" was added for a reason something like there were situations where interrupt code was executed unintentionally multiple times because the variable was not reset in between. But I cannot remember which was really the case so I'd rather trust your tests. Just a question before merging: is this supposed to be working fine for both non sleeping and sleeping nodes right? Thanks!

Ok I will try to let it run my code for a non-sleeping node and let you know if there is a change of behavior.

j54n1n avatar Sep 03 '21 08:09 j54n1n