Skip to content

Conversation

viteo
Copy link

@viteo viteo commented May 19, 2025

In my case I need to do some extra actions with direction change, so I use mcu dir_pin with external flag. But in that case the PCNT module gets "wrong" direction pin from getDirectionPin() and the pin freezes in its state.
Took some time to determine why pin freezes.
So external flag clearing solves the problem.
(tested on IDF5 but assume that IDF4 has the same problem)

@gin66
Copy link
Owner

gin66 commented May 20, 2025

Thanks for the patch. Using external pin flag to enable special processing makes sense. Still there is an issue, because the PR will make the following test for not being PIN_UNDEFINED to always succeed. In addition this works only, if the pin number with masked out external flag and GPIO are identical. This would be up to the user to guarantee. Consequently, it may be better to provide another parameter to attachToPulseCounter() with name like dirPinReadback. Default can be PIN_UNDEFINED

@viteo
Copy link
Author

viteo commented May 21, 2025

My bad, I didn't think about PIN_UNDEFINED check right the next line...

Сhanging the function parameters will require changing all demo examples (i don't have experience with arduino wrapper to test them), because they are expecting getDirectionPin() value.
I might suggest to add check inside for quick fix.

    if (dir_pin != PIN_UNDEFINED)
    {
        if(dir_pin & PIN_EXTERNAL_FLAG)
        {
            dir_pin = dir_pin & ~PIN_EXTERNAL_FLAG;
        }
        ...
    }

but this still requires user to guarantee identity. Could there be a situation where dir_pin is truly external but there is a direct pin to read direction (and why user do that)?

Thank you for your reply. I'll do as you decide.

@gin66
Copy link
Owner

gin66 commented May 21, 2025

How about changing the definition in FastAccelStepper.h to:

bool attachToPulseCounter(uint8_t pcnt_unit, int16_t low_value = -16384,
                            int16_t high_value = 16384, uint8_t dirInputPin = PIN_UNDEFINED);

@gin66
Copy link
Owner

gin66 commented May 22, 2025

Thanks for the patch so quickly. There is only one thing to do. If the parameter is PIN_UNDEFINED, then I would fall back to the default dirPin already known to the stepper. If that is undefined too or PIN_EXTERNAL_FLAG is set, then the pulse counter has no direction info and can count only unidirectional.

@viteo
Copy link
Author

viteo commented May 22, 2025

now we could check only for PIN_EXTERNAL_FLAG, but i've kept check for PIN_UNDEFINED because it's more visual

@gin66 gin66 merged commit 03f2538 into gin66:master May 22, 2025
68 checks passed
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.

2 participants