roboticslab-uc3m / yarp-devices

A place for YARP devices
https://robots.uc3m.es/yarp-devices/
9 stars 7 forks source link

Full-range PWM commands in Lacquey firmware #235

Closed PeterBowman closed 4 years ago

PeterBowman commented 4 years ago

Commit https://github.com/roboticslab-uc3m/yarp-devices/commit/cfd8d88e8555b8fd399e4ab80db95df9082e138f#diff-68eeea6c71ceb478044ee0f4dc44b6a0 (https://github.com/roboticslab-uc3m/yarp-devices/issues/211) adds a new PWM control layer around the LacqueyFetch device, resembling more closely its internal way of actuating the motor. However, our firmware is severely limited in that only full-positive (+100%), full-negative (-100%) and stop (0%) commands are forwarded, thus ignoring the whole [-1,1] value range (source):

https://github.com/roboticslab-uc3m/yarp-devices/blob/8726fa368f8b3becf26a2f263f4d451e1836eb1b/firmware/LacqueyFetch/main.cpp#L76-L92

Here, data is a char. Make it a 32-bit float so that it matches https://github.com/roboticslab-uc3m/yarp-devices/commit/38176f164de5bd0873013406efda53323c1a415c.

jgvictores commented 4 years ago

Related:

* Not prioritary, just to point out. For starters, SerialServoBoard looks PositionControl-oriented, whereas we both agreed on exposing the more native PWM interface.

jgvictores commented 4 years ago

Returning to the scope of the issue, yes, I totally agree we should recover the full range at firmware level.

rsantos88 commented 4 years ago

Updated Lacquey firmware: https://os.mbed.com/users/raulsr988/code/LacqueyFetch/rev/f31b6ddb9cd3/

rsantos88 commented 4 years ago

Note: Currently the previus firmware is installed on the Mbed. Waiting to install the latest changes (38176f164de5bd0873013406efda53323c1a415c) in Teo to be tested completely. Right now the tests have been done under Pcanview, sending messages directly to the Mbed. All works correctly. I'll close this issue

PeterBowman commented 4 years ago

Just one more thing to finally mark this as closed: merge this change into firmware/LacqueyFetch/ (also update README).