beagleboard / librobotcontrol

Robotics Focused library for embedded Linux computers. Mirror of https://git.beagleboard.org/beagleboard/librobotcontrol
https://beagleboard.org/librobotcontrol
MIT License
196 stars 158 forks source link

Fix PRU Delayed Read #138

Closed kmartin36 closed 5 years ago

kmartin36 commented 5 years ago

Only reading R31 once per loop. If R31 changes during the instructions between the reads, it creates the possibility of missing transitions. Also checking for invalid transitions where both channels transition at the same time.

StrawsonDesign commented 5 years ago

I like it, have you speed-tested this quantify the performance improvement?

Something I thought about at time of writing was to shift the position by 2 in the event both pins changed state, but this would require keeping track of current direction adding more instructions and the possibility for more unexpected behavior. Thoughts?

kmartin36 commented 5 years ago

There should not be a speed improvement. In fact, the additional check for invalid transitions adds one extra cycle to the A_CHANGED code path. That has the effect of the A_CHANGED and B_CHANGED paths taking the same number of cycles instead of the A_CHANGED taking one cycle fewer due to not executing the QBBS B_CHANGED.

The single R31 read was intended to be more robust in case the signal changes back rapidly, for example with mechanical encoders that can have issues with switch bouncing.

The current direction information is contained in the EXOR and OLD registers. After the XOR OLD, OLD, EXOR instruction, OLD is actually containing the new current value, but EXOR is still containing the changes so there is still enough information.

StrawsonDesign commented 5 years ago

Just got a change to test this with hardware. Merged manually in 920cc3e7cbda0c3c21b74108cc18d8de5b60eb4b with the compiled binary too. I know it's odd to include the binary alongside the source but it's done so the debian package can be built without the PRU build tools.