ros-industrial / ros2_canopen

CANopen driver framework for ROS2
https://ros-industrial.github.io/ros2_canopen/manual/rolling/
140 stars 60 forks source link

Reduce processor load #111

Closed hellantos closed 1 year ago

hellantos commented 1 year ago

1) lockfree::queue::wait_and_pop() is inefficient and blocking the processor. Using try_pop

2) Usually drivers need to perform some work when incoming data has changed (update loop). The data changes in the lely core executor process and the ros2 driver process should perform the processing of the changed data. Previously, this was handled by a promise - future architecture that led to many threads. Having a large number of waiting futures and threads seems to be inefficient and consuming processor load. Directly calling the processing functions from the lely core executor process creates better performance in terms of CPU usage.

For a 6 device setup with cia402 devices:

hellantos commented 1 year ago

@ipa-vsp can you verify if this also improves behavior on your PC?

ipa-vsp commented 1 year ago

Hi @ipa-cmh, The setup of 6 devices with CIA402 devices has seen a significant improvement in performance.

hellantos commented 1 year ago

I think it might make sense to have an option in bus.yml to specify if feature 2 should be activated.

Feature 2 can seriously block the canopen update loop and it should only be used, when all periodic data is fetched via pdo. It is also only useful if the bus uses sync messages.

ipa-vsp commented 1 year ago

Yes, currently, for ros2 launch canopen_tests proxy_setup.launch.py rpdo and emcy listeners are deactivated.

hellantos commented 1 year ago

Now there is the option to choose in config:

When the following is added to a node in bus.yml, an update loop in a ros2 timer will be started for the driver.

polling: true
period: 10 # in milliseconds

[or]

period: 10 # in milliseconds

When the follwing is added to bus.yml, the update loop of the driver will be triggered by CANopen Sync messages.

polling: false

This is more resource efficient, but the update loop should only work with PDO not SDO. This needs to be checked and will not always be the case, therefore option one is the default.

gsalinas commented 11 months ago

Now there is the option to choose in config:

When the following is added to a node in bus.yml, an update loop in a ros2 timer will be started for the driver.

polling: true
period: 10 # in milliseconds

[or]

period: 10 # in milliseconds

When the follwing is added to bus.yml, the update loop of the driver will be triggered by CANopen Sync messages.

polling: false

This is more resource efficient, but the update loop should only work with PDO not SDO. This needs to be checked and will not always be the case, therefore option one is the default.

@ipa-cmh This is sort of a stupid question, but this is the opposite of what's written in the documentation:

The docs say:

    • polling
    • bool
    • Enables polling of the drive status. Default: true. If false, period will be used to run a ros2 timer as update loop. If true, the update loop will be triggered by the sync signal and directly executed in the canopen realtime loop. This requires all data processed in the update loop to be PDO, otherwise the loop will get stuck. This can speed reduce processor load significantly though.

So this says, if polling is true, the update loop comes from the CAN sync, and if it's false, from a ros2 timer, but your comment is that adding polling = true will add a ROS2 timer to do the updates.

To me it makes sense that the default behavior is tied to the CANopen sync signal, so I like what the documentation says in that sense, but linguistically "polling: true" implies the ROS2 timer case to me, so I'm confused. Anyway, I just wanted to double check that the documentation is what was actually implemented and didn't accidentally get it backwards, I tried to see if there was more discussion somewhere but I didn't find anything mentioning a reversal of the meaning of what's written here.

Thank you!