Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sensors do not report values if interrupts is enabled #547

Open
j54n1n opened this issue Aug 22, 2021 · 2 comments
Open

Sensors do not report values if interrupts is enabled #547

j54n1n opened this issue Aug 22, 2021 · 2 comments

Comments

@j54n1n
Copy link

j54n1n commented Aug 22, 2021

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.

@user2684
Copy link
Contributor

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!

@j54n1n
Copy link
Author

j54n1n commented Sep 3, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants