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

RFC: Add pulseio.Counter #1423

Closed
wants to merge 1 commit into from
Closed

RFC: Add pulseio.Counter #1423

wants to merge 1 commit into from

Conversation

notro
Copy link
Collaborator

@notro notro commented Dec 23, 2018

I need pin interrupt support for a project and this is one way of achieving that if we don't end up supporting pin interrupts directly (latency is not a problem for this project since the signal is sent over i2c to a Raspberry Pi).

I have previously wondered if PulseIn (#1072) could be used, but I think a Counter abstraction is a much better fit and has other use cases too. Of course there's that feature creep syndrome and flash is scarce on some boards.

@tannewt was unsure about pull support:

I don't think the samd can pull up when EXTI is active because the pinmux is enabled.

But at least pull up is working on a samd51.

See #1415 for how this Counter can be used with async/await.

This samd-peripherals change is also necessary: https://gist.github.com/notro/2558d4fe1b0699e175c7dc33b9a74d22

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build is failing. I think that's probably due to the samd-peripherals change. Could you submit a PR to https://github.com/adafruit/samd-peripherals and then update the submodule commit so this will build? THanks.

@notro
Copy link
Collaborator Author

notro commented Dec 26, 2018

This PR isn't done yet, it lacks a way to configure pull up/down and which edge(s) to count.
It was meant for the async discussion in #1380 and shows a way to do pin interrupts with async/await through the example in #1415.

Do you think it's a good addition on its own?

If we decide to support regular pin interrupts, then I could do without this PR myself, even though this is a better fit for me than pin interrupts because a counter is interrupt storm resilient (if we do VM inline callbacks that is, or soft IRQ's as it's called).

@tannewt
Copy link
Member

tannewt commented Jan 3, 2019

I think we need something like this. I wonder though if it could be merged with FrequencyIn which should do edge counting while also tracking a period to determine frequency as well. They both can use the EXTI to generate an event which a TC can count. (Better than using an interrupt because it won't starve the CPU at high frequencies.) What do you think?

@notro
Copy link
Collaborator Author

notro commented Jan 4, 2019

Yes I'm aware of the similarity with FrequencyIn, but I need to be able to configure which edges to count on: rise/fall/both.

I'm starting to wonder if it would be better to use soft pin interrupts instead if we can just ignore the interrupt if the callback is already on the scheduler stack. This would prevent an interrupt from flooding the stack blocking other soft interrupts.

@tannewt
Copy link
Member

tannewt commented Apr 9, 2019

FrequencyIn is now checked in so please start with that if you still need this functionality. Thanks!

@tannewt tannewt closed this Apr 9, 2019
@sommersoft sommersoft mentioned this pull request Oct 11, 2019
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