-
-
Notifications
You must be signed in to change notification settings - Fork 639
shell: allow selecting SERIAL_CONTROL device #3042
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
2045ab8
962ca8a
9c87b9d
147265e
006a6bd
01364a6
0f30dcd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,72 @@ Shell::Result ShellImpl::send(std::string command) | |
| return Shell::Result::Success; | ||
| } | ||
|
|
||
| Shell::Result ShellImpl::set_device(Shell::Device device) | ||
| { | ||
| uint8_t mav_device = 0; | ||
| if (!device_to_mavlink(device, mav_device)) { | ||
| return Shell::Result::InvalidArgument; | ||
| } | ||
|
|
||
| std::lock_guard<std::mutex> lock(_device_mutex); | ||
| _device = mav_device; | ||
| return Shell::Result::Success; | ||
| } | ||
|
|
||
| bool ShellImpl::device_to_mavlink(Shell::Device device, uint8_t& out_device) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally, we don't do out parameters. You can return a pair or an optional if you need result and value.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks — switched |
||
| { | ||
| // Numeric values match MAVLink SERIAL_CONTROL_DEV (common.xml). | ||
| switch (device) { | ||
| case Shell::Device::Telem1: | ||
| out_device = 0; | ||
| return true; | ||
| case Shell::Device::Telem2: | ||
| out_device = 1; | ||
| return true; | ||
| case Shell::Device::Gps1: | ||
| out_device = 2; | ||
| return true; | ||
| case Shell::Device::Gps2: | ||
| out_device = 3; | ||
| return true; | ||
| case Shell::Device::Shell: | ||
| out_device = 10; | ||
| return true; | ||
| case Shell::Device::Serial0: | ||
| out_device = 100; | ||
| return true; | ||
| case Shell::Device::Serial1: | ||
| out_device = 101; | ||
| return true; | ||
| case Shell::Device::Serial2: | ||
| out_device = 102; | ||
| return true; | ||
| case Shell::Device::Serial3: | ||
| out_device = 103; | ||
| return true; | ||
| case Shell::Device::Serial4: | ||
| out_device = 104; | ||
| return true; | ||
| case Shell::Device::Serial5: | ||
| out_device = 105; | ||
| return true; | ||
| case Shell::Device::Serial6: | ||
| out_device = 106; | ||
| return true; | ||
| case Shell::Device::Serial7: | ||
| out_device = 107; | ||
| return true; | ||
| case Shell::Device::Serial8: | ||
| out_device = 108; | ||
| return true; | ||
| case Shell::Device::Serial9: | ||
| out_device = 109; | ||
| return true; | ||
| default: | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| Shell::ReceiveHandle ShellImpl::subscribe_receive(const Shell::ReceiveCallback& callback) | ||
| { | ||
| return _receive.callbacks.subscribe(callback); | ||
|
|
@@ -73,6 +139,11 @@ void ShellImpl::unsubscribe_receive(Shell::ReceiveHandle handle) | |
| bool ShellImpl::send_command_message(std::string command) | ||
| { | ||
| mavlink_message_t message; | ||
| uint8_t device; | ||
| { | ||
| std::lock_guard<std::mutex> lock(_device_mutex); | ||
| device = _device; | ||
| } | ||
|
|
||
| while (command.length() > MAVLINK_MSG_SERIAL_CONTROL_FIELD_DATA_LEN) { | ||
| if (!_system_impl->queue_message([&](MavlinkAddress mavlink_address, uint8_t channel) { | ||
|
|
@@ -81,7 +152,7 @@ bool ShellImpl::send_command_message(std::string command) | |
| mavlink_address.component_id, | ||
| channel, | ||
| &message, | ||
| static_cast<uint8_t>(SERIAL_CONTROL_DEV::SERIAL_CONTROL_DEV_SHELL), | ||
| device, | ||
| 0, | ||
| timeout_ms, | ||
| 0, | ||
|
|
@@ -113,7 +184,7 @@ bool ShellImpl::send_command_message(std::string command) | |
| mavlink_address.component_id, | ||
| channel, | ||
| &message, | ||
| static_cast<uint8_t>(SERIAL_CONTROL_DEV::SERIAL_CONTROL_DEV_SHELL), | ||
| device, | ||
| flags, | ||
| timeout_ms, | ||
| 0, | ||
|
|
@@ -130,6 +201,13 @@ void ShellImpl::process_shell_message(const mavlink_message_t& message) | |
| mavlink_serial_control_t serial_control; | ||
| mavlink_msg_serial_control_decode(&message, &serial_control); | ||
|
|
||
| { | ||
| std::lock_guard<std::mutex> lock(_device_mutex); | ||
| if (serial_control.device != _device) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // This adds an additional byte for the null termination. | ||
| char str_copy[sizeof(serial_control.data) + 1]{0}; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ class ShellImpl : public PluginImplBase { | |
| void disable() override; | ||
|
|
||
| Shell::Result send(std::string command); | ||
| Shell::Result set_device(Shell::Device device); | ||
| Shell::ReceiveHandle subscribe_receive(const Shell::ReceiveCallback& callback); | ||
| void unsubscribe_receive(Shell::ReceiveHandle handle); | ||
|
|
||
|
|
@@ -34,12 +35,16 @@ class ShellImpl : public PluginImplBase { | |
| private: | ||
| bool send_command_message(std::string command); | ||
| void process_shell_message(const mavlink_message_t& message); | ||
| static bool device_to_mavlink(Shell::Device device, uint8_t& out_device); | ||
|
|
||
| static constexpr uint16_t timeout_ms = 1000; | ||
|
|
||
| struct Receive { | ||
| explicit Receive(asio::io_context& io_context) : callbacks(io_context) {} | ||
| CallbackList<std::string> callbacks; | ||
| } _receive; | ||
|
|
||
| std::mutex _device_mutex{}; | ||
| uint8_t _device{10}; // SERIAL_CONTROL_DEV_SHELL | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should have the mavlink enum define available.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now using |
||
| }; | ||
| } // namespace mavsdk | ||
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.
Given this is lock is only around ever around
_deviceyou can just usestd::atomic.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.
Agreed — replaced the mutex with
std::atomic<uint8_t> _deviceand relaxed loads/stores. Pushed in 962ca8a.