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

[Pulse] Pulse Counter P003 enhancement #3482

Merged
merged 2 commits into from
Feb 19, 2021

Conversation

TD-er
Copy link
Member

@TD-er TD-er commented Jan 29, 2021

See forum topic

N.B. I created this PR for the contributor in the forum.
Not yet looked at the code changes myself.

See [forum topic](https://www.letscontrolit.com/forum/viewtopic.php?p=49935#p49935)

N.B. I created this PR for the contributor in the forum.
Not yet looked at the code changes myself.
@TD-er
Copy link
Member Author

TD-er commented Feb 18, 2021

@clumsy-stefan Since you're using the pulse counter too, can you also have a look at this PR, which contains code change from someone suggested via the forum?
I also merged in your recent changes for increasing the resolution to micros.

@clumsy-stefan
Copy link
Contributor

Sure, I'll have a look at it. the only thing that worries me after a first quick look is, that quite a lot of work is done within the IRS handling (although mostly compares and no calculations), so this could work well for low impulse rates (like energy meters) but fail on high rates (like fan's).

I'll do a version later on and check with my test fan (which can go up to 12'500 RPM).

@TD-er
Copy link
Member Author

TD-er commented Feb 18, 2021

That's why I asked you to test it :)

@clumsy-stefan
Copy link
Contributor

Had it running overnight on a couple of nodes and just now did a few tests... It seems that it's running stable and it could read my fan up to ~10'000 RPM as well in RAISINg/FALLING mode as in LOW/HIGH mode (which gave rouhgly half the cycle time, as expected). So I guess it would be OK to merge this one in as it add's the possibility to measure LOW and HIGH pulses.

@TD-er TD-er merged commit 189d18a into letscontrolit:mega Feb 19, 2021
@TD-er TD-er deleted the bugfix/p003_pulse branch February 19, 2021 17:30
@TD-er
Copy link
Member Author

TD-er commented Feb 19, 2021

Thanks for testing.

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.

None yet

2 participants