Codecov Report

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

@@           Coverage Diff           @@
##           master   #17914   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22294    22294           
=======================================
  Hits        21933    21933           
  Misses        361      361           

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

@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.

@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…

@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.

@robert-hh: sorry, I wasn't trying to teach you how to suck eggs, but I wanted to be explicit about the ESP32 behaviour with regards to clockwise vs anticlockwise turning in case the MIMXRT behaviour is/was different.

As noted, I'm not aware of any standard for 1-phase or 2-phase counting behaviour. I suspect it is arbitrary choice which edge to count on.

I honestly can't even remember where I got my original logic from. I don't think it would be particularly difficult to change if there is some feeling that the current behaviour is suboptimal. I think matching your floor-divide behaviour would just require swapping to using the reversed B signal edges for 1- and 2-phase logic.

On a side note: since support for Encoder/Counter in the Mimxrt por5 is not merged yet, this test will fail for teensy.