search

ros-controls / ros_controllers

Generic robotic controllers to accompany ros_control
http://wiki.ros.org/ros_control
BSD 3-Clause "New" or "Revised" License
574 stars 526 forks source link

seq is 0 in differential_drive controller #251

Closed Timple closed 7 years ago

Timple commented 7 years ago

If we run the differential_drive controller in open_loop mode, the seq in the headers stays 0. Although the timestamp in the header is correct.

In the code, I don't see the sequency in the header being set anywhere, is this a bug?

bmagyar commented 7 years ago

I think this is just something none of us ever needed. If you have a need for this, please feel free to provide a pull request implementing it.

On 11 January 2017 at 17:39, Tim Clephas notifications@github.com wrote:

If we run the differential_drive controller in open_loop mode, the seq in the headers stays 0. Although the timestamp in the header is correct.

In the code, I don't see the sequency in the header being set anywhere, is this a bug?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ros-controls/ros_controllers/issues/251, or mute the thread https://github.com/notifications/unsubscribe-auth/ADXH4Sf9VDAMt7jUzFSov69bo87UBztTks5rRRO8gaJpZM4Lg3Fx .

Timple commented 7 years ago

Well, it is in the header definition that it should be sequential. We have some tf problems with our robot and this sequence being zero is what stood out.

It might not be the issue, but I will make a pull request nonetheless next week to fulfill the specification of the header.

bmagyar commented 7 years ago

I cannot hurt, but keep in mind that this controller is being used on many robots in production already, has tons of hours of test time. Seq is too simple of a banana peel to slip on :)

Tf issues usually arise when time is out of sync, perhaps you have a multi computer system where you'd need to periodically sync clocks?

On 12 Jan 2017 08:24, "Tim Clephas" notifications@github.com wrote:

Well, it is in the header definition that it should be sequential. We have some tf problems with our robot and this sequence being zero is what stood out.

It might not be the issue, but I will make a pull request nonetheless next week to fulfill the specification of the header.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ros-controls/ros_controllers/issues/251#issuecomment-272102551, or mute the thread https://github.com/notifications/unsubscribe-auth/ADXH4d55SOiTBLUyfg3oUpkJ0zJ_WgpSks5rReM3gaJpZM4Lg3Fx .

mathias-luedtke commented 7 years ago

We have some tf problems with our robot and this sequence being zero is what stood out.

I doubt that tf uses the sequence number..

romainreignier commented 7 years ago

It does not hurt to have it but it seems that seq is kind of deprecated: https://discourse.ros.org/t/is-the-sequence-number-in-header-message-deprecated/1528 http://answers.ros.org/question/55126/why-does-ros-overwrite-my-sequence-number/?answer=55132#post-id-55132

bmagyar commented 7 years ago

Yeah, I'd prefer to not add support for deprecated stuff so late. I'll close both this and the referenced PR. Thanks for bringing this up though!