-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
mimxrt: Add Quadrature Encoder and Pulse Counter classes. #12347
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
base: master
Are you sure you want to change the base?
Conversation
6b55548
to
1365224
Compare
1365224
to
2ecbf50
Compare
Code size report:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12347 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 171 171
Lines 22296 22296
=======================================
Hits 21937 21937
Misses 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2ecbf50
to
d45030c
Compare
d45030c
to
508d754
Compare
508d754
to
3de0c7e
Compare
e6d0969
to
54abc06
Compare
@dpgeorge @projectgus Now that the Encoder/Counter classes are available for ESP32, I have rebased and retested the implementation for MIMXRT. Since the Encoder/Counter hardware differs between ESP32 and MIMXRT, the ESP32 oriented test in extmod_hardware show quite a few deviations. Major reasons:
|
Thanks @robert-hh for updating and noting these differences. (I think it's a good example of the potential for tests to find these hardware-specific parts as well.)
The ESP32 version of this class has a documented port-specific
I think the solution for these will probably be I am a bit surprised that |
Good day @projectgus, thank you for the swift reply. Obviously one can skip the test for the FALLING edge of the counter. I did not look in detail why the Pin value() cannot read back when the Pin was configured for Encoder/Counter. Probably it's because for Encoder/Counter a PIN is configured for a different ALT mode. So maybe the test has to be changed to configure the Pin for machine.Pin mode first, make the connectivity test and then configure it as Encoder/Counter. I wondered why there is the connectivity test at all. The other tests to not have it. |
So I added an emulation for phases and adapted the test scripts, taking into account the not-supported options and the fact, that different to ESP32 the MIMXRT expects 4 transitions to count from 0 to 1. For ESP32, setting the initial value of pulses to -1 fixes that. |
b6eb32c
to
d115ab6
Compare
Sorry I didn't reply last week, @robert-hh - the updated PR looks great to me, thanks for accommodating the limits in the API and tests! I don't think I have any mimxrt board set up to do any local tests on, unfortunately (there might be a Teensy 4 hiding somewhere, will look later).
I borrowed this from Ihor's test PR, but it's something I support building into tests - especially hardware-in-loop tests. The idea is to have a quick and user-friendly way to differentiate "the test environment is broken" from "the driver is broken". More than once I've had some unusual pattern of failures and spent too long debugging them before realising a wire is loose, or some equivalent prerequisite is missing (i.e with MicroPython, forgetting to do As you observe, MicroPython's tests don't have much of this kind of thing (yet) - but I think that's to the project's detriment. It costs time even for core developers (like me) who run the test suite a lot, but for developers who aren't already very familiar with the project then it's even more limiting. We get a lot of PRs from new(ish) contributors who are competent developers, but the relevant tests either haven't been run, or they ran and failed and the developer hasn't dug in to figure out why. (Sorry for dropping in this somewhat tangential rant. cc @hmaerki and @dpgeorge as I know they've been thinking about tests an awful lot, lately.) In the particular case of the connections test for Counter/Encoder, I think skipping them on mimxrt - as you've done here - is fine for now. Long term, putting them in a separate TestCase at the top of the same file (so they're run without the setUp step that inits the Encoder and muxes the pins) will probably allow them to pass on more platforms, but I can look at that in a follow-up if you like. |
That's what #16112 is trying to address, although it's gotten pretty lost among all the other PRs. Might be nice to get that merged, I use it all the time now.
We are definitely missing a simple But also, I think we need to break out all this board wiring configuration into separate files, one for each board, that can be reused in all hardware-related tests. That's something I'm working on now. |
I'm not familiar with the mechanics of unittest, so I do not object if you look at that later. |
These classes are base on the Quadrature Encoder blocks of the i.MXRT MCUs. The i.MXRT 102x has two encoders, the other ones four. The i.MXRT 101x does not support this function. It is implemented as two classes, Encoder and Counter. The number of pins that can be uses as inputs is limited by the MCU architecture and the board schematics. The Encoder class supports: - Defining the module - Defining the input pins. - Defining a pin for an index signal. - Defining a pin for a Home signal. - Defining an output pin showing the compare match signal. - Setting the number of cycles per revolution. - Setting the initial value for the position. - Setting the counting direction. - Setting a glitch filter. - Setting the value counter as signed or unsigned integers. - Defining callbacks for getting to a specific position, overrun and underrun (starting the next revolution). These callbacks can be hard interrupts to ensure short latency. The encoder counts all phases of a cycle. The span for the position is 2**32, for the revolution is 2**16. The highest input frequency is CPU-Clock/24. The Counter mode counts single pulses on input A of the Encoder. The configuration support: - Defining the module - Defining the input pin. - Defining the counting direction, either fixed or controlled by the level of an input pin. - Defining a pin for an index signal. - Defining an ouput pin showing th compare match signal. - Setting the counter value. - Setting the glitch filter. - Returing the value counter as signed or unsigned integer. - Defining a callback which is called at a certain value. The counting range is 0 - 2**32-1 and a 16 bit overrun counter. The highest input frequency is CPU-Clock/12. Signed-off-by: robert-hh <robert@hammelrath.com>
Signed-off-by: robert-hh <robert@hammelrath.com>
The MIMXRT1015 MCU has only one encoder/counter unit. Signed-off-by: robert-hh <robert@hammelrath.com>
This is the MIMXRT specific version, compliant to the documentation of PR micropython#8072 in the basic methods. Signed-off-by: robert-hh <robert@hammelrath.com>
Signed-off-by: robert-hh <robert@hammelrath.com>
Signed-off-by: robert-hh <robert@hammelrath.com>
and replace "STATIC" with "static". Signed-off-by: robert-hh <robert@hammelrath.com>
For Teensy 4.x. The connectivity tests and the falling edge of the counter test are skipped. Signed-off-by: robert-hh <robert@hammelrath.com>
By dividing the internal counter by 1, 2 or 4. The default is 4, matching the hardware. Signed-off-by: robert-hh <robert@hammelrath.com>
d115ab6
to
5d33af4
Compare
This PR adds Quadrature Encoder and Pulse Counter classes based on the Encoder hardware of the mimxrt MCUs. The base methods are as simple as possible, with options to make use of the hardware features supporting fast encoder sensors, like index pulses.
Tested with a slow manual encoder and a simulation of fast encoder/counter signals up to a input frequency of 25 MHz.
The PR is a re-submit of PR #7911, which could not be reopened. The conversation at that PR applies here as well (except for the UART part).