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

Noise filtering #16

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Noise filtering #16

wants to merge 3 commits into from

Conversation

kirill9617
Copy link

@kirill9617 kirill9617 commented Nov 24, 2016

I had noise with pulse duration greater than signed int and this patch fixes my problem.

@mwittig
Copy link
Contributor

mwittig commented Nov 24, 2016

Can you please squash your commits into a single commit? Thanks

@mwittig
Copy link
Contributor

mwittig commented Dec 1, 2016

I think the root problem is an overflow with the calculation of "duration" at https://github.com/pimatic/RFControl/blob/master/RFControl.cpp#L290

duration is an unsigned int (16bit) value while currentTime (and lastTime) hold 32 bit values. This will overflow in particular in the first cycle when lastTime is 0, but it will also overflow at longer intervals ... 65536 microsecs * 4 = if the interval is longer than 200 ms roughly.

Another thing to consider is timer rollover, which happens roughly every 50 days if I calculated it correctly. http://playground.arduino.cc/Code/TimingRollover

@sweetpi
Copy link
Contributor

sweetpi commented Dec 11, 2016

@kirill9617 Nice catch. As @mwittig mentioned: I think the pull request does not fix the problem itself.

We should check that the result of (currentTime - lastTime) / PULSE_LENGTH_DIVIDER does fit into a unsigned int (16bit). If it does not fit then we could skip the pulse.

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

Successfully merging this pull request may close these issues.

3 participants