ros2_control PDO commands #219

macstepien · 2024-01-17T17:23:18Z

New capabilities were introduced in fw 2.1a for Roboteq, which allowed us, among others to use PDO commands, and move some other information also to PDO. Additional required changes to panther_msgs: husarion/panther_msgs#18

Conflicts: panther_hardware_interfaces/README.md panther_hardware_interfaces/include/panther_hardware_interfaces/canopen_controller.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/panther_system.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/roboteq_data_converters.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/roboteq_driver.hpp panther_hardware_interfaces/src/motors_controller.cpp panther_hardware_interfaces/src/panther_system.cpp panther_hardware_interfaces/src/roboteq_driver.cpp

…be triggered)

Conflicts: panther_bringup/launch/bringup.launch.py panther_controller/config/WH01_controller.yaml panther_controller/config/WH02_controller.yaml panther_controller/config/WH04_controller.yaml

Conflicts: panther_controller/config/WH01_controller.yaml panther_controller/config/WH02_controller.yaml panther_controller/config/WH04_controller.yaml panther_description/urdf/panther_macro.urdf.xacro panther_hardware_interfaces/CMakeLists.txt panther_hardware_interfaces/CODE_STRUCTURE.md panther_hardware_interfaces/README.md panther_hardware_interfaces/include/panther_hardware_interfaces/canopen_controller.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/motors_controller.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/panther_system.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/panther_system_ros_interface.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/roboteq_data_converters.hpp panther_hardware_interfaces/include/panther_hardware_interfaces/roboteq_driver.hpp panther_hardware_interfaces/src/canopen_controller.cpp panther_hardware_interfaces/src/motors_controller.cpp panther_hardware_interfaces/src/panther_system.cpp panther_hardware_interfaces/src/panther_system_ros_interface.cpp panther_hardware_interfaces/src/roboteq_driver.cpp

KmakD

Again, PR without tests

panther_description/urdf/panther_macro.urdf.xacro

panther_hardware_interfaces/src/roboteq_driver.cpp

panther_hardware_interfaces/src/motors_controller.cpp

panther_hardware_interfaces/README.md

panther_hardware_interfaces/config/roboteq_motor_controllers_v80_21a.eds

panther_hardware_interfaces/README.md

panther_hardware_interfaces/src/roboteq_driver.cpp

pkowalsk1

I'm satisfied with these changes; they are just a few suggestions.

How about using "" instead of <> when including certain header files?

panther_hardware_interfaces/README.md

panther_hardware_interfaces/include/panther_hardware_interfaces/motors_controller.hpp

pkowalsk1 · 2024-02-05T22:26:30Z

panther_hardware_interfaces/include/panther_hardware_interfaces/motors_controller.hpp

  const RoboteqData & GetFrontData() { return front_data_; }
  const RoboteqData & GetRearData() { return rear_data_; }


Suggested change

const RoboteqData & GetFrontData() { return front_data_; }

const RoboteqData & GetRearData() { return rear_data_; }

const RoboteqData & GetFrontDataRef() { return front_data_; }

const RoboteqData & GetRearDataRef() { return rear_data_; }

I believe we discussed this ref/copy issue previously, but I just suggest adding Ref to the method names as a solution. What are your thoughts?

I would prefer the regular names (without Ref), how about you @KmakD?

Don't have a strong opinion about this. I guess with Ref we leave less room for confusion later

To be honest, I would even prefer to remove reference and just copy, if that can be confusing, instead of adding Ref to method names xd I think that it will look strange, and I haven't seen such convention anywhere.

pkowalsk1 · 2024-02-05T23:02:52Z

panther_hardware_interfaces/src/roboteq_driver.cpp

 bool RoboteqDriver::Boot()
 {
  booted_.store(false);
-  return FiberDriver::Boot();
+  return LoopDriver::Boot();
 }

 bool RoboteqDriver::WaitForBoot()


I'm just curious about what happens when we use std::future<std::string> in this boot sequence. In my opinion, it would be clearer, but I am aware of the fact that it may not be worth it. Using std::async() in CANopenController::BootDrivers() and remove WaitForBoot() could be an option there ;)

I'm not sure if async can be used in this case - Boot method only requests boot operation, once it completes OnBoot is called, for which WaitForBoot waits. It could be done with future, but I wouldn't use string for that - IMO that would look worse. Instead bool variable can be used, that will store result if the boot was successful or not (change booted_ to promise). This makes sense, not sure if it will be that better, but I can change it. What do you think about it?

Sounds good. @KmakD ?

Sounds good to me too

Done, 488c578, it wasn't even necessary to use bool promise.

panther_hardware_interfaces/src/roboteq_driver.cpp

panther_hardware_interfaces/test/src/roboteq_mock.cpp

macstepien · 2024-02-06T09:50:17Z

I'm satisfied with these changes; they are just a few suggestions.

How about using "" instead of <> when including certain header files?

I just mentioned it yesterday when reviewing GPIO and I agree. This change will have to be done also in other parts of the code ('battery', 'lights').

macstepien added 30 commits December 7, 2023 09:58

Update eds to fw21a and change it to match new sent data

1317094

Change commands to pdos, update sdo operations and update reading pdos

efb5ec3

Change reading driver state to pdo and update pdo remapping

8ff57bb

New pdo configuration

169d5e0

Add heatsink temperature

0d2f142

Update whole system to use new pdo communication and add proper timeouts

a1a3729

Remove old gpio driver and temporarily comment out tests

dbc63d4

Rename eds file

0165f34

Add configurable driver states update frequency

2c427cd

Fix spinning in panther system ros interface

6cddcdc

Fix destroying objects

a943378

Switch to loop driver (better performance)

b136720

Update PDO driver state timeout log

14ac66a

Fix destroying canopen controller

d0e64d6

Change frequency to 125hz

ada9fec

Change return failure to error (in this cases on_error method should …

eda3ef2

…be triggered)

Update log messages

6eaa96c

Update volts amps and battery names

5e120d2

Update ignored runtime errors

d2ab015

Update documentation

57c57b3

Fix naming and update documentation

b1ff7af

Change to 100Hz and increase allowed number of errors

a46a66d

Use sdo operation timeout parameter

dc7b036

Remove additional timeout in sdo operations

3fbf827

Remove comment and todos

e040720

Update controllers config

a49d134

Remove todos

75844db

Update readme

1ffc035

Update communication parameters

91c00e0

macstepien marked this pull request as draft January 18, 2024 15:31

macstepien added 5 commits January 29, 2024 14:07

Merge branch 'ros2-devel' into ros2-control-pdo-commands

44cd3ca

Conflicts: panther_bringup/launch/bringup.launch.py panther_controller/config/WH01_controller.yaml panther_controller/config/WH02_controller.yaml panther_controller/config/WH04_controller.yaml

Update tests

215b011

Add std to int types

e5648c9

Update coment

eda691c

macstepien requested review from KmakD and pkowalsk1 January 30, 2024 10:30

Fix comment in the urdf

a540cf2

KmakD requested changes Jan 31, 2024

View reviewed changes

macstepien added 2 commits January 31, 2024 16:12

CR suggestions

1ce8aaa

Refactor panther system

61fc921

macstepien requested a review from KmakD January 31, 2024 16:47

KmakD requested changes Feb 1, 2024

View reviewed changes

panther_hardware_interfaces/README.md Outdated Show resolved Hide resolved

panther_hardware_interfaces/README.md Outdated Show resolved Hide resolved

panther_hardware_interfaces/src/roboteq_driver.cpp Outdated Show resolved Hide resolved

CR suggestions - readme fixes

38e1056

macstepien requested a review from KmakD February 1, 2024 12:50

pkowalsk1 reviewed Feb 5, 2024

View reviewed changes

macstepien added 4 commits February 6, 2024 09:56

CR suggestions - update readme

7bb1d6e

CR suggestions - move flags reading to a separate variable

55a2493

CR suggestions - move roboteq mock methods implementation

2818653

CR suggestions - change to lock guard and fix locking range

5484f45

macstepien requested a review from pkowalsk1 February 6, 2024 12:34

macstepien marked this pull request as ready for review February 6, 2024 12:47

macstepien added 2 commits February 6, 2024 16:34

CR suggestions - use future in roboteq driver boot

488c578

Refactor tests

6755c34

pkowalsk1 approved these changes Feb 8, 2024

View reviewed changes

KmakD approved these changes Feb 8, 2024

View reviewed changes

macstepien merged commit 724a99a into ros2-control Feb 8, 2024

macstepien deleted the ros2-control-pdo-commands branch February 8, 2024 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ros2_control PDO commands #219

ros2_control PDO commands #219

macstepien commented Jan 17, 2024 •

edited

Loading

KmakD left a comment

pkowalsk1 left a comment

pkowalsk1 Feb 5, 2024 •

edited

Loading

macstepien Feb 6, 2024

KmakD Feb 6, 2024

macstepien Feb 6, 2024

pkowalsk1 Feb 5, 2024 •

edited

Loading

macstepien Feb 6, 2024

pkowalsk1 Feb 6, 2024

KmakD Feb 6, 2024

macstepien Feb 6, 2024

macstepien commented Feb 6, 2024

		const RoboteqData & GetFrontData() { return front_data_; }
		const RoboteqData & GetRearData() { return rear_data_; }

ros2_control PDO commands #219

ros2_control PDO commands #219

Conversation

macstepien commented Jan 17, 2024 • edited Loading

KmakD left a comment

Choose a reason for hiding this comment

pkowalsk1 left a comment

Choose a reason for hiding this comment

pkowalsk1 Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

macstepien Feb 6, 2024

Choose a reason for hiding this comment

KmakD Feb 6, 2024

Choose a reason for hiding this comment

macstepien Feb 6, 2024

Choose a reason for hiding this comment

pkowalsk1 Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

macstepien Feb 6, 2024

Choose a reason for hiding this comment

pkowalsk1 Feb 6, 2024

Choose a reason for hiding this comment

KmakD Feb 6, 2024

Choose a reason for hiding this comment

macstepien Feb 6, 2024

Choose a reason for hiding this comment

macstepien commented Feb 6, 2024

macstepien commented Jan 17, 2024 •

edited

Loading

pkowalsk1 Feb 5, 2024 •

edited

Loading

pkowalsk1 Feb 5, 2024 •

edited

Loading