Skip to content

esp32: Add machine.Counter and machine.Encoder. #12346

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

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

Conversation

jimmo
Copy link
Member

@jimmo jimmo commented Aug 31, 2023

This builds on the extensive work done by @IhorNehrutsa and @jonathanhogg (with contributions from many others including @robert-hh and @harbaum). Thank you to everyone involved!!

It combines the design + documentation in PR #8072 with the implementation in PR #7582. This allows us to provide the full-featured interface to the ESP32 hardware, while also providing a lightweight wrapper providing the simplified portable Counter and Encoder classes.

I note that compared to the implementation in #8766 this does not support setting IRQs on the Encoder and Counter classes, however this is still possible with the esp32.PCNT class (and I think we should add support for this to Encoder and Counter anyway).

Also, as @jonathanhogg has pointed out, this uses the now-deprecated IDF pcnt driver, but updating it to the new one isn't that difficult. @projectgus suggests that we should consider moving to the HAL instead, as the driver doesn't offer a significant benefit.

I have made the following additional changes:

esp32.PCNT:

  • Remove the ability to set a value, instead just a reset flag. (The hardware doesn't support it, and requiring the value being set to zero is a bit awkward). (The Encoder and Counter classes still support setting a value though).
  • Minor code cleanups and commenting (especially around some of the more trickier aspects around avoiding race conditions with IRQ handlers).
  • Remove the auto-assigning of unit IDs. We don't do this for other peripherals.

machine.Counter:

  • Simplify the way __new__ and __init__ work to implement the singletons.
  • Make the id passed to Counter (and Encoder) correspond directly to the PCNT id. This makes Encoder and Counter share the same "namespace" which I think is natural considering they do use the same underlying resource.
  • Make it count overflows rather than pulses, so that it can count for longer before overflowing small integer. This is important to avoid needing to allocate in the IRQ handler. This also changes the way we handle setting a value (and I think also fixes a race condition).
  • Comments to explain the subtleties in avoiding race conditions.

machine.Encoder:

  • Didn't previously work for phases=1 or 2. (Due to the way the pin arguments were handled in PCNT).
  • Make phases=1 the default (not 4). We should provide the same default on all ports, and a port may not be able to provide 2x or 4x quadrature.

Docs:

  • Minor clarifications to ESP32-specific support & availability.
  • Explain the overflow and small integer behavior.

Other:

  • Make this enabled by default on OG,S2,S3 (rather than explicitly disabling on C3).
  • Use the "new" sys.path trick for having the frozen machine.py extend the built-in one, rather than import umachine. Also use a lazy-loading getattr (similar to what we do elsewhere) to avoid needing to copy the entire machine locals dict into RAM.
  • Tested up to 1MHz on ESP32.

I look forward to seeing this implemented on other ports! @robert-hh I hope it should be relatively straightforward to merge #7911 now.

Although much of the work was done by the people already mentioned, I should mention that my work on this was funded through GitHub Sponsors (as well as some time for @projectgus and @dpgeorge... we spent a lot of time discussing this). As always, this is much appreciated and very helpful for getting these more involved bits of work reviewed and merged. Sorry for how long it's taken for us to get to this and the lack of communication from us, it's a very useful feature!

@robert-hh
Copy link
Contributor

robert-hh commented Aug 31, 2023

I hope it should be relatively straightforward to merge #7911 now.

Thanks for the work. I did not expect any more that these classes were considered. Then I can re-open PR 7911 again. I had closed it to avoid the git message noise while keeping it vivid.
Edit: I cannot reopen it, so I'll re-submit the PR.

@jonathanhogg
Copy link
Contributor

jonathanhogg commented Aug 31, 2023

Don't get me wrong, it's going to be great to see a counter/encoder API based on the PCNT hardware finally make it into mainline, but I've spent over 2 years supporting PR #7582 without any engagement from the core team. I'd have happily taken API/implementation input at any point over that time and made the changes necessary to get it merged.

A "sorry for […] the lack of communication" is nice to hear, but I don't feel like it addresses a key issue: how does MicroPython maintain the good will of a contributing community if it is unable even when funded to find the time to engage with those developers. This does not feel like a strategy for growing the pool of MicroPython developers.

I guess what I'm saying is that I probably ought to be pleased to see my code finally approaching mainline, but instead I just feel deflated.

[Edit: For the benefit of posterity, Jim and I have been in contact out-of-band and I have apologised for the tone of this comment. Leaving this here as-is as a reminder to myself to take a breath before hitting send…]

@IhorNehrutsa
Copy link
Contributor

Also, as @jonathanhogg has pointed out, this uses the now-deprecated IDF pcnt driver, but updating it to the new one isn't that difficult. @projectgus suggests that we should consider moving to the HAL instead, as the driver doesn't offer a significant benefit.

I've also been looking at a new IDF5 PCNT driver and agree with @projectgus that we should consider switching to the HAL.

@IhorNehrutsa
Copy link
Contributor

Don't get me wrong, it's going to be great to see a counter/encoder API based on the PCNT hardware finally make it into mainline, but I've spent over 2 years supporting PR #7582 without any engagement from the core team. I'd have happily taken API/implementation input at any point over that time and made the changes necessary to get it merged.

A "sorry for […] the lack of communication" is nice to hear, but I don't feel like it addresses a key issue: how does MicroPython maintain the good will of a contributing community if it is unable even when funded to find the time to engage with those developers. This does not feel like a strategy for growing the pool of MicroPython developers.

I guess what I'm saying is that I probably ought to be pleased to see my code finally approaching mainline, but instead I just feel deflated.

I completely agree with @jonathanhogg and confirm every word he says.
This is generally terrible and outrageous.
I have enough PR without any reaction from maintainers for days, weeks, months, and years.
Sorry, couldn't resist.

@jimmo
Is it possible to select a person from maintainers to support the ESP32 port?

@jimmo jimmo force-pushed the esp32_machine_counter branch from 0ae56c7 to 7ace9a2 Compare August 31, 2023 14:23
@andrewleech
Copy link
Contributor

Thanks for putting this together @jimmo, consolidating new machine api's in a way that should be compatible across many ports is a seriously non-trivial task.

@jonathanhogg & @IhorNehrutsa I'm personally disappointed by negativity your responses, I thought the leading paragraphs of acknowledgments for everyone's contributions here were quite respectful and it's an exciting feature to get progress on. I don't mean to dismiss your feelings but in text form it reads really harsh to me.

I've got a lot of open pr's too. Many of these I also maintain and rebase regularly - because I want to use them and get personal value from them. micropython doesn't owe me anything for doing this.

Sure, it'd be nice if every one of them got detailed commenting and direction and rapid merging - but this is still a very small company / project from a funding and staff perspective, responding meaningfully on every issue, pr, discussion, discord, etc would be multiple people's full time job.
Speaking from experience, writing detailed feedback describing significant refactoring let alone combine multiple people's work together is a lot of work. Certainly at work to guide others or train new developers it's usually worth it, but it's expensive.

I regularly find it's much quicker to describe my sw architecture vision in code... so in a small resource constrained team I understand it can be far more efficient to combine and update things yourself, with appropriate author attribution that should be a win-win.

I understand that community development needs communication to foster growth and to bring everyone closer together.
However another thing that helps community immensely is for the people who submit pr's to also code review other pr's and assist in resolving issues, this can seriously reduce the load on the core team! Some of the people on this thread do a huge amount of this already... I and others could do more.

I recently turned on full notifications for this repo and it gave me a lot of perspective here... I suggest you try it yourself and watch the email stream of all issues/pr/discussion updates for a few days.
It gave me a feeling for just how crazy active this project is and the mammoth amount of responding, guiding and feedback the core team are already doing. Then turn on full notifications for discord too if you're still not convinced.
I have no idea how they balance their time developing vs communicating, there's just so much going on!

Side note; in my opinion the only way there could / should ever be a dedicated maintainer for esp port is of Espressif funded them full time directly and permanently, and this person was hand picked by Damien to be the right person for the job as the esp stuff still needs to be compatible and integrated with the other ports as much as possible.

@vahithosan
Copy link

vahithosan commented Sep 2, 2023

Hi. I'm trying to test the counter.

I want to pulse with RMT and count with counter.

Example :

from machine import UART, Pin, Timer, PWM, ADC
import time
import micropython
import esp32
from machine import Pin
from esp32 import PCNT

srv = esp32.RMT(0, pin=Pin(18), clock_div=4)

sayac=0

def k():
    global sayac
    sayac=sayac+1
    print("counter: ", sayac)


counter = PCNT(0, pin=Pin(19), minimum = 0 , maximum = 32000, threshold0 = 5000, rising=PCNT.INCREMENT)        # create counter
counter.irq(handler=k(), trigger= counter.IRQ_THRESHOLD0)

counter.start()   

When I write this :
counter.irq(handler=k(), trigger= counter.IRQ_THRESHOLD0)
k() function works.

The k() function does not work when threshold0 is reached. What am I doing wrong?

@jonathanhogg
Copy link
Contributor

At a quick glance, it should just be:

counter.irq(handler=k, trigger=counter.IRQ_THRESHOLD0)

You are calling the handler and passing its return value to .irq() instead of passing a reference to the function itself.

@vahithosan
Copy link

vahithosan commented Sep 2, 2023

At a quick glance, it should just be:

counter.irq(handler=k, trigger=counter.IRQ_THRESHOLD0)

You are calling the handler and passing its return value to .irq() instead of passing a reference to the function itself.

Thank you very much
I added argument to the function.

def k(c):
    global sayac
    sayac=sayac+1
    print("counter: ", sayac)

and

>>> srv.write_pulses(5000*(640,640), 0)
>>> counter:  1

@IhorNehrutsa
Copy link
Contributor

I expect that this PR is aimed at industrial encoders with a resolution of thousands of pulses per revolution and thousands of rotations per minute.
image

For rotary encoders with push switch
image
may be used MicroPython modules like
MicroPython-quadrature-incremental-encoder
or Encoders from @peterhinch

@jonathanhogg
Copy link
Contributor

I see no reason not to use built-in hardware support for reading simple hand-turn encoders.

Not just that, but I routinely use the Counter() class with simple push switches. I get a debounced interrupt-driven object that I can simply query in my main loop to see if the button has been pushed since the last iteration, a la:

button = Counter(0, Pin(0), filter_ns=12500)

if button.value(0):
    ...

Even the very short maximum filter window provided by PCNT is sufficient in my experience for debouncing decent buttons.

@IhorNehrutsa
Copy link
Contributor

I see no reason not to use built-in hardware support for reading simple hand-turn encoders.

I have no objection to such use of the Counter() and Encoder().


Call this method with no arguments to return the current counter value.

If the optional *reset* argument is set to ``True`` then the counter is
Copy link
Member

Choose a reason for hiding this comment

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

This feels inconsistent with other machine APIs: if a method is acting like a getter and setter then the setter should take the value to set it to, which in this case would be 0, not True.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was the primary reason why I originally went with .value(0) for resetting the counter, as it allowed for a (near-)atomic read-and-reset while fitting with the setter convention.

Copy link
Member Author

Choose a reason for hiding this comment

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

The intention with the change is that having the ability to set the value but only allowing the set value to be zero is a bit weird. What I wanted to convey is that the "action" is reset/clear, not "set to value". It would probably make sense if we required this to be a kwarg-only... e.g. pcnt.value(reset=True).

Either way, I don't care that much, I've reverted it back to just value-that-must-be-zero. (And same for init).

Copy link
Contributor

Choose a reason for hiding this comment

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

reset=True as a keyword-only argument makes sense to me. You're right that making .value() a combined getter/setter feels weird when you can only set it to 0 – honestly, I was never particularly comfortable with that bit of my API. I think that having .value(0) return the previous value is already stretching the setter convention a bit thin…

Copy link
Contributor

@IhorNehrutsa IhorNehrutsa Oct 3, 2023

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

In the general case .value(some_non_zero_value) is possible.

Yes, that is still supported on machine.Counter and machine.Encoder. This limitation only applies to the underlying esp32.PCNT class which doesn't manage any internal counter state.


``trigger`` should be the desired events or'ed together. The ``handler``
function should take a single argument which will be a bit mask indicating
which counter event(s) occurred.
Copy link
Member

Choose a reason for hiding this comment

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

This is not how other machine irq methods work. The "standard" (see eg rp2's Pin.irq) is for the irq handler to take a single argument being the instance of the original peripheral object, in this case the PCNT instance.

Also, the irq() method should return a special IRQ object which can be used to get the flags indicating which events occurred. This is all implemented by the common shared/runtime/mpirq.c code (which isn't as widely used as it should be...).

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't even realise that callback objects supported reading the triggered event! This doesn't seem to be well documented (the esp32 machine.Pin.irq() docs say nothing more than "This method returns a callback object"), and I hadn't realised there was common code for supporting it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a little bit awkward to make this work with the standard mechanism, although I agree that we should make it consistent. (And yes, currently nothing in the esp32 port uses mpirq.c).

I've updated the PR with an attempt at making this work with mpirq.c. This requires that irq.flags() clears the flags on read, which is different to other implementations, but is a requirement to implement PCNT correctly.

There's a subtle thing to be aware of here which is that when calling pcnt.value(), when it ensures that the IRQs have run, it requires that the handler calls pcnt.irq().flags() to zero them, otherwise this will be an infinite loop. As the code stands right now, you can't tell whether the flags are still non-zero because the user didn't call irq.flags() or whether the ISR has run again in the meantime. Of course we could solve this by having another state variable to track "has the ISR run", but I'm not sure if this is necessary?

There's also an issue here (which I think was also true in the original version) which is that if you call pcnt.value() in the irq handler, then it can recursively call the irq handler. I will think about that more tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the fact that .value() can call the IRQ handler is pretty ugly, but I couldn't figure out an easy way to make the logic for Counter() race-proof otherwise. Perhaps just zero the flags after the handler has been called instead? So .flags() would be valid and static within the request handler.

I was pretty sure at the time that a call to .value() within the request handler wouldn't cause a loop in my original code, but I'd have to diff your PR against my original branch and then stare at it a bit to see if that still holds.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using mpirq.c also means we can no longer use the static scheduler node.

It's not super obvious to me how to add support for using static scheduler nodes to mpirq.c without having a node-per-instance (i.e. one per PCNT).

Copy link
Contributor

Choose a reason for hiding this comment

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

Using mpirq.c also means we can no longer use the static scheduler node.

Bah! All bets are off then 😉

Copy link
Member Author

@jimmo jimmo Oct 3, 2023

Choose a reason for hiding this comment

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

Perhaps just zero the flags after the handler has been called instead?

That was my first thought, but unfortunately this isn't possible with mpirq.c because it calls the handler for you. What you'd need to do is to have a wrapper handler (owned by esp32_pcnt_obj_t) which the handler (owned by esp32_pcnt_irq_obj_t) called (which then reset the flags).

But this still doesn't work because the low-level ISR might update the flags in the meantime.

This might be a good opportunity to re-evaluate how mpirq.c works -- in other places where we use mpirq.c I think it would benefit from changing to this behavior. e.g. in rp2/machine_pin.c the gpio_irq function always sets the flags, so for example a RISING will clobber a pending FALLING. (Edit: to be clear, by "changing to this behavior" I mean the behavior currently in this PR where irq.flags() is clear-on-read and have the ISR always OR into them).

I was pretty sure at the time that a call to .value() within the request handler wouldn't cause a loop in my original code, but I'd have to diff your PR against my original branch and then stare at it a bit to see if that still holds.

I did see #7582 (comment) but I think https://github.com/micropython/micropython/pull/7582/files#r1015829258 is more relevant (and I think here you're saying that it can cause a loop?). If the ISR runs (and sets more flags) then it will keep recursing. Like you say, this isn't a problem if the irq handler method is re-entrant (which it should be?), and it's also exceptionally unlikely to happen in practice (also now it's confined to a single unit).

Bah! All bets are off then 😉

Indeed. Especially if you have multiple PCNT units running concurrently. I'll leave this one for @dpgeorge to comment before I start modifying mpirq.c to support static nodes. (I really don't want to duplicate the functionality mpirq.c in esp32_pcnt.c just to have a single shared static node).

Copy link
Contributor

Choose a reason for hiding this comment

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

I did see #7582 (comment) but I think https://github.com/micropython/micropython/pull/7582/files#r1015829258 is more relevant (and I think here you're saying that it can cause a loop?)

Ah, yes. What I figured was that if a second event occurred immediately after starting the call to the handler, but before a call to .value() within the handler, that would cause a further nested call to the handler, but it could only recurse further if interrupts kept happening.

So an interrupt storm could conceivably cause you to run out of stack, but it couldn't get into an infinite loop. Of course, nested calls to the handler function might cause some other ugly behaviour in the user code…

I was planning to put more thought into how to avoid this, but my brain started to hurt.

Maybe the real problem here is the large-number implementation of Counter(), which requires an offset that is kept in careful sync with the hardware counter. This behaviour could, perhaps, be pushed back into PCNT() itself where more careful interrupt handling could be done in C.

esp32_pcnt_obj_t *pcnt = MP_STATE_PORT(esp32_pcnt_obj_head);
while (pcnt != NULL) {
uint32_t status = pcnt->pending;
pcnt->pending ^= status; // TODO should this disable IRQs?
Copy link
Member

Choose a reason for hiding this comment

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

Does this TODO need to be resolved, or can the comment be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I've convinced myself the TODO is resolved.

Add a new `esp32.PCNT` class that provides complete, low-level support to
the ESP32 PCNT pulse counting hardware units.

This can be used as a building block to implement the higher-level
`machine.Counter` and `machine.Encoder` classes.

This is enabled by default on all OG, S2, S3 boards, but not on C3 (as the
PCNT peripheral is not supported).

Original implementation by: Jonathan Hogg <me@jonathanhogg.com>

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@jimmo jimmo force-pushed the esp32_machine_counter branch from 7ace9a2 to 65a2b0b Compare October 3, 2023 13:07
Document the new `esp32.PCNT` class for hardware pulse counting.

Originally authored by: Jonathan Hogg <me@jonathanhogg.com>

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Adds a Python override of the `machine` module, which delegates to the
built-in module and adds an implementation of `Counter` and `Encoder`,
based on the `esp32.PCNT` class.

Original implementation by: Jonathan Hogg <me@jonathanhogg.com>

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Add documentation for `machine.Counter` and `machine.Encoder` as currently
implemented by the esp32 port, but intended to be implemented by other
ports.

Originally authored by: Ihor Nehrutsa <Ihor.Nehrutsa@gmail.com> and
Jonathan Hogg <me@jonathanhogg.com>.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@jimmo jimmo force-pushed the esp32_machine_counter branch from 65a2b0b to 27a53df Compare October 3, 2023 13:23
from machine import Pin, Encoder

counter = Counter(0, Pin(0, Pin.IN), Pin(1, Pin.IN)) # create Encoder for pins 0, 1 and begin counting
value = counter.value() # retrieve current count
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to be the correct example? Only the comment on line 15 is different to the machine.Counter.rst example?

karlp added a commit to karlp/helloween2 that referenced this pull request Oct 14, 2023
Fixes all positioning woes, you just need a custom micropython build :)
But hey, we're doing that already!
Needs: micropython/micropython#12346
@harbaum
Copy link

harbaum commented Dec 5, 2023

May I ask what the state of this is? I have been using @jonathanhogg's personal branch for quite some time now (many thanks for providing it for so long). But it seems he has given up supporting it which is sad but fully understandable.

Is there a PR or branch that is considered to be "the" implementation of PCNT/Counter for micropython? Even if it's not merged (yet)? This one?

@harbaum
Copy link

harbaum commented Dec 5, 2023

I've applied this PR to tag v1.21.0. It applies cleanly, but the resulting code panics as soon as i use the counter:

from machine import Pin, Counter
counter = Counter(0, Pin(26), filter_ns=10000)

Gives:
Guru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled.

@projectgus
Copy link
Contributor

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

@dpgeorge dpgeorge added this to the release-1.24.0 milestone Jul 17, 2024
@oyooyo
Copy link

oyooyo commented Sep 23, 2024

I just compiled Micropython with this PR included because I needed a pulse counter.
I'm getting the same error that @harbaum mentioned, a "Guru Meditation error" when trying to instantiate the Counter class.

After looking a little further into this, the error seems to occur when the Counter class constructor calls pcnt.init() with keyword argument value=0. Here's a minimal example that reproduces the error:

from esp32 import PCNT
pcnt = PCNT(0)
pcnt.init(value=0)

The error does not seem to occur when passing value=0 to the PCNT class constructor directly:

from esp32 import PCNT
pcnt = PCNT(0, value=0)

@jonathanhogg
Copy link
Contributor

I just compiled Micropython with this PR included because I needed a pulse counter.

I resurrected my original PR #7582, on which this was based, recently as I needed a pulse counter again and this PR has sadly stagnated. You could try using that code. I have not done extensive testing of it, but am happy to provide support if it doesn't work.

@oyooyo
Copy link

oyooyo commented Sep 23, 2024

I've worked around the problem by writing my own minimalistic Counter class that does not use pcnt.init(value=0), so for now I'm fine and just wanted to give my feedback on this PR.

But thanks for the tip, @jonathanhogg. I believe Micropython really needs a Pulse Counter, so if this PR here has bugs and is no longer maintained then I hope your PR will be merged instead. Since this PR is based on yours the API is probably similar, so switching to your pulse counter implementation should be easy for me.

karlp added a commit to karlp/micropython that referenced this pull request Nov 6, 2024
needed for micropython#12346 after
micropython updates
karlp added a commit to karlp/micropython that referenced this pull request Nov 6, 2024
From micropython#12346 (comment)
we see that there's a bug in counter init.  Verified it crashed before,
and doesn't crash any more.

Tested with a basic button counter, used filter_ns setting as well.

Signed-off-by: Karl Palsson <karl.palsson@marel.com>
@karlp
Copy link
Contributor

karlp commented Nov 6, 2024

I've patched this per #12346 (comment) and it works, but it's still missing quite a lot? Does anyone have an alternative that can support getting notified when counter thresholds are hit? I can work on it, but was hoping it already existed :)

(fixes on top of this PR, if you want to play with things: karlp@7f4c608 and karlp@9de8935)

@jonathanhogg
Copy link
Contributor

Does anyone have an alternative that can support getting notified when counter thresholds are hit? I can work on it, but was hoping it already existed :)

I cannot the remember the specifics of this PR, but it's roughly based on my one and so I thought it did support interrupts on thresholds?

As already noted, my original PR #7582 exists and I have recently rebased it to v1.24.

@karlp
Copy link
Contributor

karlp commented Nov 6, 2024

Does anyone have an alternative that can support getting notified when counter thresholds are hit? I can work on it, but was hoping it already existed :)

I cannot the remember the specifics of this PR, but it's roughly based on my one and so I thought it did support interrupts on thresholds?

As already noted, my original PR #7582 exists and I have recently rebased it to v1.24.

right, I was looking only at the "Counter()" module, now that I'm looking at the esp32.PCNT module, it does what I wanted :)

@IhorNehrutsa
Copy link
Contributor

Please rebase it

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

Successfully merging this pull request may close these issues.