Skip to content

Conversation

IhorNehrutsa
Copy link
Contributor

Add pin initial level.
Add test Encoder phases x2 and x4.

Explanation to the #12346 (comment)

Output waveform of industrial quadrature encoders:
image

Output waveform of rotary encoders is slightly different:
image
They have several fixed positions per revolution with channel A and channel B output levels equal to 1.
I suspect that @jonathanhogg, @jimmo used exactly this kind of encoder.

He has high levels of input channels and increment the Encoder on the falling edge of channel A.

falling=self._PCNT.INCREMENT,

So, the esp32 test requires initial high levels of channels before starting the count.

Also added mimxrt configuration from the #12347
@robert-hh You can cherry-pick from here too.

@IhorNehrutsa IhorNehrutsa force-pushed the fix_only_3_pulses_to_count_first_rotation branch from d153043 to 80ddee0 Compare August 13, 2025 15:38
Add pin initial level.
Add test Encoder phases x2 and x4.

Signed-off-by: Ihor Nehrutsa <Ihor.Nehrutsa@gmail.com>
@IhorNehrutsa IhorNehrutsa force-pushed the fix_only_3_pulses_to_count_first_rotation branch from 80ddee0 to cf658f0 Compare August 13, 2025 15:46
Copy link

codecov bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (744270a) to head (cf658f0).
⚠️ Report is 17 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17914   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22295    22295           
=======================================
  Hits        21936    21936           
  Misses        359      359           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Aug 14, 2025
@projectgus projectgus self-requested a review August 14, 2025 01:02
@dpgeorge dpgeorge added tests Relates to tests/ directory in source and removed extmod Relates to extmod/ directory in source labels Aug 15, 2025
Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

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

Thanks @IhorNehrutsa for the explanation and the updated test. This looks good to me.

@robert-hh do you have any concerns about these changes?

@@ -13,12 +13,22 @@
import sys
from machine import Pin

PRINT = False
PIN_INIT_VALUE = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

The test fails if this is set to any other value, yes? If so, I think it's OK to hard-code it at line 46

@robert-hh
Copy link
Contributor

@robert-hh do you have any concerns about these changes?

I have, indeed. It works with ESP32. It fails with MIMXRT. The previous test version worked. The reason is about what should be the result of the encoder value with different settings of phases. Looking at a few test lines:

        self.assertPosition(0)
        self.rotate(1)
        self.assertPosition(1, 1, 1)
        self.rotate(1)
        self.assertPosition(1, 1, 2)
        self.rotate(1)
        self.assertPosition(1, 2, 3)
        self.rotate(1)
        self.assertPosition(1, 2, 4)  # +4

The tests sets up three instances with phases 1, 2, and 4. After one pulse, the test expects all three counter to advance by one. So the expected position is (1,1,1). The way the phases emulation is implemented in MIMXRT, the result is (0,0,1). MIMXRT counts in the positive range at phases=2 at the last one of every 2 transitions, and likewise for phases=1 in the last of every 4 transitions.ESP32 seems to advance the counter at the first of every 2 resp 4 transition block. The phases emulation can be changed, that's not a problem. but what it the proper behavior. Counting before an event is finished like the ESP32 does seems wrong to me. But it may be the behavior defined in a standard. Do you know any reference?

P.S. And yes, setting the PIN_INIT_VALUE = 0 causes the ESP32 to fail the test as well.

@jonathanhogg
Copy link
Contributor

@robert-hh:

Best way to think about the current ESP32 logic is to divide the diagram into four edges, t1 to t4, from left-to-right. In the clockwise direction, those edges are passed through t1 > t2 > t3 > t4. In the anti-clockwise direction, they are passed through t4 > t3 > t2 > t1.

The 1-phase logic is to always count on t1. That means the start of the step in the clockwise direction and the end of the step in the anti-clockwise direction. The 2-phase logic is to count on t1 and t3 – so a similar difference depending on which direction you're turning. This is because the logic counts only on edges of the A signal. For 4-phase logic, a second counter channel is used to count on both edges of the B signal as well.

I guess this sort of makes sense if you think about the counting occurring at a place rather than a time: if I very carefully turn the encoder in one direction and then back again, the increment and decrement will happen at exactly the same physical position.

Staring at the setup, I think it would be possible to count on the later edge in either direction for 1- and 2-phase logic, but it would require using both channels and a more complicated configuration. I have no idea whether there is any "right" answer to which edges one should count on with 1- and 2-phase logic. I can find no particular reference. Perhaps because anyone who cares actually uses 4-phase logic…

@robert-hh
Copy link
Contributor

@jonathanhogg I do of yourse understand the diagram at the first post. My question is, whether this behavior is a kind of standard and all implementations must match that, or is it the ESP32 implementation and other vendors may implement it differently.

The MIMXRT hardware implements solely phase=4. So the emulation would divide the internal counter value by 2 or 4 to emulate phases=2 or phases=1 And I added now offsets to meet the expected behavior of this specific test script.

@projectgus Speaking of a connection check. The MIMXRT encoder hardware allows to get the actual state of the input signals. That could be uses as a means to check the connection, if these values are exposed with an API.

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

Successfully merging this pull request may close these issues.

5 participants