-
Notifications
You must be signed in to change notification settings - Fork 13.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add offboard mode switch module for simplifying offboard for ros2 #22530
base: main
Are you sure you want to change the base?
Conversation
617d053
to
e34dd74
Compare
This commit adds a offboard mode switch module to enable using dedicated topics for offboard mode
e34dd74
to
09824b8
Compare
@Jaeyoung-Lim The idea here is that the new module monitors the |
@beniaminopozzan Yes! |
Great, is there a reason for having only the logic for trajectory, attitude and rate setpoint? what about thrust and torque and direct? |
@beniaminopozzan No real reason, just wasnt able to test with it yet One thing missing from this is sanity checking the combination of setpoints (e.g. you cant set thrust and torque with rates at the same time) |
Co-authored-by: Matthias Grob <[email protected]>
Co-authored-by: Daniel Agar <[email protected]>
Co-authored-by: Matthias Grob <[email protected]>
@MaEtUgR we are still waiting on your review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The is no offboard_thrust_setpoint
, offboard_torque_setpoint
and offboard_actuator_motor_setpoint
yet
I suggest to mark the PR as draft until it is ready
@beniaminopozzan I have intentionally left them out, since I think for those I think it might be just better to publish directly to the actuators due to the time delay introduced by publishing/republishing. I also wouldnt know how to test it if we have the interface other than SITL, which is not representative of doing this on hardware. If we intend to add such interface, I would do it in a separate PR |
@Jaeyoung-Lim oh I see, that makes sense unless the expected increase in latency is proven wrong by real hardware tests. Nevertheless, this further increase the complexity of the offboard interface, now users have to keep in mind that some offboard setpoints have to be paired with the offboardControlMode messages while other setpoints don't. Wouldn't it be cleaner at this point, instead of having a centralized offboard mode swither, to distribute this on every module that uses the "setpoint" messages? Conficting modes situations will be addressed at the single controller level, which will check |
I have been trying to avoid this, as by doing this we would create a new "flight mode" for offboard mode and all controller modules need to be aware of the "offboard" interface. For me the value of offboard mode is that the control modules do not know whether the vehicle is in offboard mode and therefore we can test individual structures by simply turning on and off different levels of control handled by commander. If somebody is implementing a new controller, they do not need to implement a special "offboard" interface to test it. Regarding completeness, I agree with you that it would be nice to support direct actuator setpoints. However, this PR solves a problem where a lot of people are struggling to use the offboard interface due to the "handshaking" behavior it requires(need to monitor vehicle flight mode before publishing the setpoints). The people struggling to do this are rarely people using direct actuator inputs. On the other hand, I do not have the capacity to test the direct actuator setpoint interface for this PR. So if anyone would be interested in contributing it would be great. But otherwise, it will be hard to include it as part of this PR. Note that this PR does not interfere/replace the previous way of engaging offboard mode. It just adds a simplified interface to test it. |
Solved Problem
When using offboard mode with ros2, the sender needs to be careful to not publish the setpoints while not in offboard mode.
This can be achieved typically by subscribing to a vehicle status message, and publishing the setpoints only when the vehicle enters offboard mode.
However, if the person implementing the offboard mode is not responsible, this may be prone to errors by intefering with other controllers.
Fixes #22512
@ayhamalharbat FYI
Solution
Changelog Entry
For release notes:
Alternatives
We could also keep the interface transparent, and require the ros2 node to be responsible for making sure that it does not interfere with other topics
Test coverage
Tested in Gazebo, with
gz_omnicopter