Expose Cui reset encoder command in firmware

PeterBowman commented 4 years ago

The CuiAbsolute firmware tree contains two applications: while Pic_source is used in normal operation, Cui_zero targets the encoder reset process (store a new zero-angle value). Any time we need to reset a Cui, this latter program needs to be loaded to the offending encoder (by plugging a serial connector to the encoder itself), run, and replaced again by the Pic_source program.

In this issue, I propose to merge Cui_zero into Pic_source and enable the reset encoder command so that it's understood by the encoder similarly to other available functions (start/stop push mode, poll). Note, however, that making it easily available entails the hazard of accidentally resetting a Cui. Such command was already being handled some time ago on the YARP side, and commented out apparently for this very reason:

https://github.com/roboticslab-uc3m/yarp-devices/blob/380480966fd16bed8c279c6fc32d241799bf9f08/libraries/YarpPlugins/CuiAbsolute/ICanBusSharerImpl.cpp#L108-L122

Assuming this command is merged into Pic_source, the next options arise regarding the CAN master:

Expose it over the YARP network via yarp::dev::IEncoders::resetEncoder (easy to call via RPC: set enc 0).
Expose it over the YARP network via yarp::dev::IRemoteVariables (buggy, but harder to call unintentionally).
Just write a script to generate and send a conveniently formed message over the CAN network (needs to be launched on the TEO PC).

Please share your opinions and votes.

PS: new label discussion was added to this repo.

PeterBowman commented 4 years ago

Idea: expose it via IEncoders, but don't enable this method unless this device is instantiated with a specific YARP option.

jgvictores commented 4 years ago

Idea: expose it via IEncoders, but don't enable this method unless this device is instantiated with a specific YARP option.

Maybe via a protectEncoders/readOnlyEncoders property that can be modified via IRemoteVariables?

PeterBowman commented 4 years ago

Maybe via a protectEncoders/readOnlyEncoders property that can be modified via IRemoteVariables?

If we need to enable/disable the zero command on runtime, that's the way to go. Not sure it is worth the trouble, though, since routing IRemoteVariables calls through the CanBusControlboard tree (+TechnosoftIpos, since CuiAbsolute will be instantiated from within this one) implies that a specific .ini configuration is picked from the start, anyway.

rsantos88 commented 4 years ago

Expose it over the YARP network via yarp::dev::IEncoders::resetEncoder (easy to call via RPC: set enc 0).

In my opinion, a RPC command: set enc n v (where nshould be the number of the encoder and v, the value in degrees) could be used to to set a particular value of the relative encoder at a required position, adding an offset to it. This set value can be returned to its original value (the same as the absolute encoder), restarting canBusControlBoard

Another thing would be to rewrite the CUI to zero position. The second option: Expose it over the YARP network via yarp::dev::IRemoteVariables (buggy, but harder to call unintentionally) would be interesting.

jgvictores commented 4 years ago

Protecting via IRemoteVariables was just an idea. Since we don't have the mapping figured out, imho an option at init would be more than enough.

PeterBowman commented 4 years ago

Thank you for your inputs! More comments originating from a face-to-face with @jgvictores:

A script is even more than enough, we'd rather issue a cansend command (via apt install can-utils) with a properly constructed message (preferably something a bit more convolute than a single byte) to perform the encoder reset. Document this at yarp-devices-tricks or similar.
Options regarding IEncoders::setEncoder and resetEncoder:
- don't implement them at all <- preferred for laziness and less trouble
- implement, but guard behind a boolean that is only set on init when passing a specific YARP configuration option
- just implement and forget, it is the responsibility of the wrapping TechnosoftIpos device to use or not to use any CuiAbsolute's methods
TechnosoftIpos::setEncoder would only take effect on the relative encoders (it already works this way). Same for the IMotorEncoders::resetMotorEncoder implementation (not done AToW, see https://github.com/roboticslab-uc3m/yarp-devices/issues/157).
We want to store absolute encoder readings on device init (https://github.com/roboticslab-uc3m/yarp-devices/issues/217) so that the resulting offset value is added to the relative encoder reads on runtime. It's certainly a bit less of a trouble to call setEncoder on init to synchronize relative and absolute encoders instead. There is no need to involve a local variable (which should be properly set/reset by setEncoder in that scenario).

PeterBowman commented 4 years ago

We want to store absolute encoder readings on device init (#217) so that the resulting offset value is added to the relative encoder reads on runtime. It's certainly a bit less of a trouble to call setEncoder on init to synchronize relative and absolute encoders instead. There is no need to involve a local variable (which should be properly set/reset by setEncoder in that scenario).

See https://github.com/roboticslab-uc3m/yarp-devices/issues/217#issuecomment-533786340.

rsantos88 commented 4 years ago

We are using sendAck(n) where nis:

0x100 -> start/stop push confirmation
0x180 -> pull confirmation
0x?? -> set zero confirmation

what value do you recommend me to use in sendAck function?

PeterBowman commented 4 years ago

what value do you recommend me to use in sendAck function?

I'd say 0x200.

rsantos88 commented 4 years ago

https://github.com/roboticslab-uc3m/yarp-devices/commit/73b3d00c7ce99716518941ff2a2a0a11001e24c4: Added set zero functionality

rsantos88 commented 4 years ago

One of the things we wanted to achieve was to be able to set the zero of the CUIs using some command to send the corresponding message by CAN. I've tested this functionality with pcanview, one of the utilities included with the driver. I want to warn you that I've updated the driver and the utilities of the Peak to the latest version 8.9.0, previously version 8.5.1 installed on teo-PCs. I could see that the test utilities contain new parameters and pcanview breaks less (I've personally verified this). With this utility we can send the desired CAN packets to the PICs, as well as see their responses (ACK messages) or position messages. In addition, as you can see, the IDs can be seen in decimal for more convenience. In this video you see a test with the PIC 126. I've checked the three commands: PUSH, STOP and PULL. Finally, from second 50 I move the joint and then I set the encoder to zero, using the 0xFF message. I've read from it to check the new value. Everything works correctly.

rsantos88 commented 4 years ago

I propose the possibility of adding a README.md here with the information on how to use pcanview to set an encoder to zero

rsantos88 commented 4 years ago

Fixed ID wrong value (115b3f948f334297ac93394c40a23328f76bd741)