Dds goal pub scripting dependency #29014
Conversation
| @@ -176,6 +176,8 @@ class AP_Vehicle : public AP_HAL::HAL::Callbacks { | |||
| virtual bool start_takeoff(const float alt) { return false; } | |||
| // Method to control vehicle position for use by external control | |||
| virtual bool set_target_location(const Location& target_loc) { return false; } | |||
| // Get target location for use by external control | |||
| virtual bool get_target_location(Location& target_loc) { return false; } | |||
There was a problem hiding this comment.
Every function called by DDS that was only enabled by scripting should always be moved into this block.
Otherwise, if you disable scripting and enable DDS, you can't compile. The reason it wasn't moved earlier was oversight. We could catch this in a build with scripting disabled and DDS enabled.
|
I was expecting to see a patch to Without that patch this is not NFC. Is that intentional? |
The PR today was not following our previous patterns. Upon further inspection, I realized this is the proper patch IF we want to follow the current patterns. DDS does not depend on scripting. They should always be independent, and AP should compile with neither enabled, with either enabled, or both enabled. I do not think making DDS depend on scripting is a good idea. |
There was a problem hiding this comment.
Changes also required to AP_DDS_Client.h and AP_DDS_Topic_Table.h.
See: https://github.com/srmainwaring/ardupilot/tree/dds-goal-pub-scripting-dependency.
(tried force pushing changes but permission denied).
Looks ok with the additional change. Tested build and execution on esp32empty with DDS enabled.
…ternal control Signed-off-by: Ryan Friedman <[email protected]>
…al control Signed-off-by: Ryan Friedman <[email protected]>
Ryanf55
force-pushed
the
dds-goal-pub-scripting-dependency
branch
from
ba5cfe1 to
778604e
Compare
#29009 (comment)
Tests Performed:
A bit back on
master, specifically on commit 87b0de7, if you tried that, you got:Now, it compiles as expected.