ros-industrial / robotiq

Robotiq packages (http://wiki.ros.org/robotiq)
BSD 2-Clause "Simplified" License
232 stars 382 forks source link

Strings as commands for FT300 #92

Closed NikolasE closed 7 years ago

NikolasE commented 7 years ago

The current way to send commands to the FT300 is via strings in a service. To zero the sensor, one has to send a "SET ZRO" (Not to be confused with "SET ZERO"). This leads also to parsing code like

strncpy(get_or_set, &buff[0], 3); strncpy(nom_var, &buff[4], strlen(buff) -3);

What do you think about a srv format like this:

uint8 COMMAND_GET_SERIAL_NUMBER=1
uint8 COMMAND_GET_FIRMWARE_VERSION=2
uint8 COMMAND_GET_PRODUCTION_YEAR=4
uint8 COMMAND_SET_ZERO=8
uint8 command
---
bool success
string res

This leads to much cleaner code on both sides and the client does not have to remember string-commands.

(I volunteer to implement the changes)

shaun-edwards commented 7 years ago

@NikolasE, thanks for the suggestion. I think this would indeed be an improvement. With this kind of change, we have to be careful to think about backward compatibility. We wouldn't want to break any code that is currently working with the string based service. I believe there are few options:

What do you think? I think the second option may be the most user friendly and maintainable.

NikolasE commented 7 years ago

The second option looks ok. What do you think about a ROS_WARN if the command-string is not empty?


uint8 COMMAND_GET_SERIAL_NUMBER=1
uint8 COMMAND_GET_FIRMWARE_VERSION=2
uint8 COMMAND_GET_PRODUCTION_YEAR=4
uint8 COMMAND_SET_ZERO=8
uint8 command_id
string command  # deprecated, please use command_id with a value of COMMAND_*
---
bool success
string res
shaun-edwards commented 7 years ago

Addressed in #93