sbgisen / vesc

VESC Interface for ROS
Apache License 2.0
44 stars 33 forks source link

Bad reading of current and temperature #28

Closed spajder16 closed 3 years ago

spajder16 commented 3 years ago

Abstract

Hi, I have a problem with reading three parameters from /sensors/core topic. The temperature_pcb, current_motor and current_input gives bad values. vesc_problem

I use FSESC6.6 Plus with firmware 5.2 version.

Is it normal and it is a bug in code with buffer interpreting or is it something in my setup?

ssr-yuki commented 3 years ago

Hi. As you say, current vesc_driver failed to read temperature and current values from newer firmware. It is a bug. I doubt the data map of VESC information packet has changed, but I have not got any information about it.

I want help on this issue.

spajder16 commented 3 years ago

Hi, I have tried https://github.com/f1tenth/vesc and there is a similar problem, but there the other values are wrong. Never the less in this other repo the currents are correct so I've tried to find the difference, but it seems that you are doing the same thing in functions like VescPacketValues::current_motor() / VescPacketValues::getMotorCurrent()

spajder16 commented 3 years ago

I think that I found the problem. I've printed out the frame_ and the payloadend and this is what I've get: frame_: 2 73 4 1 93 1 39 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 151 0 0 0 10 0 0 0 0 0 0 0 163 0 0 0 14 0 0 10 52 0 0 13 166 0 21 89 171 224 111 0 0 0 0 0 0 255 255 255 254 255 255 255 255 127 143 3 payloadend: 0 0 0 0 0 0 208 8 0 0 113 127 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 151 0 0 0 10 0 0 0 0 0 0 0 163 0 0 0 14 0 0 10 52 0 0 13 166 0 21 89 171 224 111 0 0 0 0 0 0 255 255 255 254 255 255 255 255

as you can see the payloadend has some weird 12 first bytes which don't exist in frame_. This is the problem because in those 12 bytes is temp and current. I don't know jet how to fix it, but maybe you will have some idea.

spajder16 commented 3 years ago

I've changed the payloadend.first to frame_.begin() +2 and now it's working. I think it is not the best solution and maybe you will come up with a better one.

double VescPacketValues::readBuffer(const uint8_t map_id, const uint8_t size) const
{
  int32_t value = 0;
  switch (size)
  {
    case 2:
      value += static_cast<int32_t>(*(frame_.begin() + 2 + map_id) << 8);
      value += static_cast<int32_t>(*(frame_.begin() + 2 + map_id + 1));
      break;
    case 4:
      value += static_cast<int32_t>(*(frame_.begin() + 2 + map_id) << 24);
      value += static_cast<int32_t>(*(frame_.begin() + 2 + map_id + 1) << 16);
      value += static_cast<int32_t>(*(frame_.begin() + 2 + map_id + 2) << 8);
      value += static_cast<int32_t>(*(frame_.begin() + 2 + map_id + 3));
      break;
  }

  return static_cast<double>(value);
}

vesc_problem_solved

ssr-yuki commented 3 years ago

@spajder16 Great information! Many thanks! I could reproduce to get correct current data by replacing payload_end_.first to frame_.begin() + 2 as you show.

I guess that the difference comes from copy-constructor of VescFrame class used in: https://github.com/sbgisen/vesc/blob/001a86c390fd68ab04270e0dd081e31b939b24a7/vesc_driver/src/vesc_packet.cpp#L111

The copy never updates payload_end_ with current frame_. I conclude that updating payload_end_ after the copy would resolve this issue.