Skip to content
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.

Sign up for GitHub

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

Jump to bottom

GCS_MAVLink: GCS_Common: Add support to MAV_CMD_DO_SET_SYS_CMP_ID #26424

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+47 −1
Open

GCS_MAVLink: GCS_Common: Add support to MAV_CMD_DO_SET_SYS_CMP_ID #26424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d24536
modules: mavlink: Update to latest version
patrickelectric Mar 12, 2024
63c1e3c
GCS_MAVLink: GCS_Common: Add support to MAV_CMD_DO_SET_SYS_CMP_ID
patrickelectric Mar 6, 2024
924dff8
AP_HAL: Scheduler: Add late_reboot
patrickelectric Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions libraries/AP_HAL/Scheduler.cpp
View file
Open in desktop
Expand Up @@ -6,6 +6,16 @@ using namespace AP_HAL;

extern const AP_HAL::HAL& hal;


void Scheduler::late_reboot() {
hal.scheduler->thread_create(FUNCTOR_BIND_MEMBER(&Scheduler::do_late_reboot, void) , "scheduler", 2048, AP_HAL::Scheduler::PRIORITY_MAIN, 0);
}

void Scheduler::do_late_reboot() {
hal.scheduler->delay(1000);
hal.scheduler->reboot();
}

void Scheduler::register_delay_callback(AP_HAL::Proc proc,
uint16_t min_time_ms)
{
Expand Down
2 changes: 2 additions & 0 deletions libraries/AP_HAL/Scheduler.h
View file
Open in desktop
Expand Up @@ -71,6 +71,8 @@ class AP_HAL::Scheduler {
virtual bool is_system_initialized() = 0;

virtual void reboot(bool hold_in_bootloader = false) = 0;
void late_reboot();
void do_late_reboot();

/**
optional function to stop clock at a given time, used by log replay
Expand Down
1 change: 1 addition & 0 deletions libraries/GCS_MAVLink/GCS.h
View file
Open in desktop
Expand Up @@ -597,6 +597,7 @@ class GCS_MAVLINK
#endif

MAV_RESULT handle_do_set_safety_switch_state(const mavlink_command_int_t &packet, const mavlink_message_t &msg);
MAV_RESULT handle_do_set_sys_cmp_id(const mavlink_command_int_t &packet, const mavlink_message_t &msg);

// reset a message interval via mavlink:
MAV_RESULT handle_command_set_message_interval(const mavlink_command_int_t &packet);
Expand Down
29 changes: 29 additions & 0 deletions libraries/GCS_MAVLink/GCS_Common.cpp
View file
Open in desktop
Expand Up @@ -5173,6 +5173,32 @@ MAV_RESULT GCS_MAVLINK::handle_do_set_safety_switch_state(const mavlink_command_
}
}

#if AP_MAVLINK_MAV_CMD_DO_SET_SYS_CMP_ID_ENABLED
MAV_RESULT GCS_MAVLINK::handle_do_set_sys_cmp_id(const mavlink_command_int_t &packet, const mavlink_message_t &msg)
peterbarker marked this conversation as resolved.
Show resolved Hide resolved
{
const uint8_t new_system_id = packet.param1;
const uint8_t new_component_id = packet.param2;
const uint8_t do_reboot = packet.param3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a "reboot" or "this change should take effect immediately" flag?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a reboot flag. https://mavlink.io/en/messages/development.html#MAV_CMD_DO_SET_SYS_CMP_ID

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we read the message description.

The intent is for the thing to take effect immediately, 'though, isn't it? We can probably do that without a reboot my modifying the mavlink state variables.

I think this field should probably be "take effect immediately", rather than "reboot". We can discuss at DevCallEU tonight if you like....


if (new_system_id == 0 || hal.util->get_soft_armed()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we need to reject the command if new_system_id is zero (apart from "that's what the message says")

This means you can't tell a system to just change its component ID.

... but you can tell it to just change its system ID. This is weird.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire explanation of the message logic is here: mavlink/mavlink#2082 (comment)

In a system with multiple components per vehicle, accepting a broadcast will change all components to a single component_id, and that would be terrible. The only way to change the component ID is to communicate with it directly. You can change all components to the same system_id though.

return MAV_RESULT_DENIED;
}

if (!AP_Param::set_and_save_by_name_ifchanged("SYSID_THISMAV", new_system_id)) {
return MAV_RESULT_FAILED;
}

if (new_component_id != 0 && !AP_Param::set_and_save_by_name_ifchanged("COMPID_THISMAV", new_component_id)) {
return MAV_RESULT_FAILED;
}

if (do_reboot) {
peterbarker marked this conversation as resolved.
Show resolved Hide resolved
hal.scheduler->late_reboot();
}

return MAV_RESULT_ACCEPTED;
}
#endif

MAV_RESULT GCS_MAVLINK::handle_command_int_packet(const mavlink_command_int_t &packet, const mavlink_message_t &msg)
{
Expand Down Expand Up @@ -5334,6 +5360,9 @@ MAV_RESULT GCS_MAVLINK::handle_command_int_packet(const mavlink_command_int_t &p
case MAV_CMD_DO_SET_SAFETY_SWITCH_STATE:
return handle_do_set_safety_switch_state(packet, msg);

case MAV_CMD_DO_SET_SYS_CMP_ID:
return handle_do_set_sys_cmp_id(packet, msg);

#if AP_MAVLINK_SERVO_RELAY_ENABLED
case MAV_CMD_DO_SET_SERVO:
case MAV_CMD_DO_REPEAT_SERVO:
Expand Down
4 changes: 4 additions & 0 deletions libraries/GCS_MAVLink/GCS_config.h
View file
Open in desktop
Expand Up @@ -42,6 +42,10 @@
#define AP_MAVLINK_MAV_CMD_REQUEST_AUTOPILOT_CAPABILITIES_ENABLED 1
#endif

#ifndef AP_MAVLINK_MAV_CMD_DO_SET_SYS_CMP_ID_ENABLED
#define AP_MAVLINK_MAV_CMD_DO_SET_SYS_CMP_ID_ENABLED 1
#endif

#ifndef HAL_MAVLINK_INTERVALS_FROM_FILES_ENABLED
#define HAL_MAVLINK_INTERVALS_FROM_FILES_ENABLED ((AP_FILESYSTEM_FATFS_ENABLED || AP_FILESYSTEM_POSIX_ENABLED) && BOARD_FLASH_SIZE > 1024)
#endif
Expand Down
2 changes: 1 addition & 1 deletion modules/mavlink
Open in desktop
Submodule mavlink updated 3 files
+6 −0 message_definitions/v1.0/ardupilotmega.xml
+18 −1 message_definitions/v1.0/common.xml
+13 −0 message_definitions/v1.0/development.xml