Add detach async policy for rate critical frameworks #2477
Conversation
github-actions
bot
requested review from
bijoua29,
bmagyar,
destogl,
mcbed,
qiayuanl,
rosterloh and
VX792
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2477 +/- ##
==========================================
- Coverage 89.45% 89.44% -0.01%
==========================================
Files 147 147
Lines 16636 16679 +43
Branches 1400 1405 +5
==========================================
+ Hits 14881 14919 +38
- Misses 1218 1219 +1
- Partials 537 541 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| hardware.is_async = parse_is_async_attribute(ros2_control_it); | ||
| hardware.thread_priority = hardware.is_async ? parse_thread_priority_attribute(ros2_control_it) | ||
| : std::numeric_limits<int>::max(); | ||
| hardware.async_params.thread_priority = hardware.thread_priority; |
There was a problem hiding this comment.
Maybe we should add this to migration_notes
There was a problem hiding this comment.
I left the old one too. Should I add it to the notes still?
There was a problem hiding this comment.
We need this one for sure. I removed it by mistake. This is a fallback approach when you don't define it in the properties tag
| if (info_.type != "sensor") | ||
| if ( | ||
| !is_sensor_type && | ||
| this->get_lifecycle_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE) |
There was a problem hiding this comment.
is this intented here? How does this work together with #2501?
There was a problem hiding this comment.
Yes, it is intended. It is the same as when we have an INACTIVE component, we only call read callback but not the write
There was a problem hiding this comment.
yes which means that we don't allow the "configuration" commands in inactive?
|
This pull request is in conflict. Could you fix it @saikishor? |
For frameworks such as EtherCAT that run at high frequency. if the deadline is not met, then the master will start showing errors and eventually it might lead to non-functional system. For this reason, instead of the previous approach where the CM acts as a scheduler for the async activity, I've added a possibility for the component to manage it's own cycle
Needs: ros-controls/realtime_tools#383
Needs: ros-controls/realtime_tools#402