dynamixel-workbench icon indicating copy to clipboard operation
dynamixel-workbench copied to clipboard

Improper casting?

Open swinterbotix opened this issue 5 years ago • 3 comments

Setup is done using an Ubuntu Linux laptop to a U2D2. Currently using the XM540-W270-T and XM430-W350-T servos - however, I think this question would apply to any Dynamixel X-Series servo. Using Protocol 2.0 at 1 Mbps.

I'm currently using the itemRead function available through the Dynamixel Workbench. The issue I am having is that if I use this function to read from the 'Present_PWM' or 'Present_Current' registers when the values of these registers should be negative, I get values that don't make sense (for example, 65526). I feel like this might be an improper casting issue due to using an uint16_t somewhere...

My code is something like this.

const char* log; bool result = false; int32_t value;

result = dxl_wb.itemRead(1, "Present_Current", &value, &log);

Again, if the Current value should be positive, then 'value' shows the correct value. However, if the Current value should be negative, then 'value' shows a value like 65526. Same goes for the 'Present_PWM' register.

swinterbotix avatar Jan 13 '20 20:01 swinterbotix

I ran into a similar issue.
The "Control Table of RAM Area" details the byte size of "Data Name". "Present Current" is 2 bytes, so use an int16_t. Hope this helps.

tj60647 avatar Feb 19 '20 06:02 tj60647

But the itemRead function (and the readRegister function it calls) expects a pointer to an int32_t as specified at https://github.com/ROBOTIS-GIT/dynamixel-workbench/blob/master/dynamixel_workbench_toolbox/src/dynamixel_workbench_toolbox/dynamixel_workbench.cpp#L251?

Also, if you look at the source code for the readRegister function at https://github.com/ROBOTIS-GIT/dynamixel-workbench/blob/master/dynamixel_workbench_toolbox/src/dynamixel_workbench_toolbox/dynamixel_driver.cpp#L824, it looks like the data from the 'Present_Current' register is placed in a 'uint16_t' type (which doesn't sound right to me as the 'Goal_Current' register can be negative). It then appears that the value is implicitly casted to an in32_t before the function returns. So I don't see how using an int16_t will fix this...

swinterbotix avatar Feb 19 '20 19:02 swinterbotix

So I did a bit more digging - it seems that the

  1. syncRead getData function (from the DynamixelSDK c++ API) returns a uint32_t
  2. the readRegister function (from the DynamixelDriver) stores the return value temporarily in either a uint8_t, uint16_t, or uint32_t before implicitly converting it into a int32_t type.
  3. the itemRead function (from the DynamixelWorkbench class) returns a int32_t

So if I use the syncRead, readRegister, or itemRead functions to get Present PWM or Current, and the values are negative, you will get values like 65535.

However, the DynamixelWorkbench has a function called convertValue2Current that takes in a int16 - and outputs the correct current even if they are negative. So somehow, the int32 storing the raw and incorrect register value for negative PWM or current - when it gets implicitly cast to an int16 - works just fine. Instead of putting the responsibility to do this on the user, I think it would be better for the getData and readRegister functions to properly handle negative register values before returning data. I think this could be done just by checking a register's data type - casting to that data type - then cast again to an int32 and return the value.

swinterbotix avatar Jul 21 '20 21:07 swinterbotix