-
Notifications
You must be signed in to change notification settings - Fork 111
core: services: ardupilot_manager: mavlink_proxy: Fix MAVLink Server component and system ID #3592
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
core: services: ardupilot_manager: mavlink_proxy: Fix MAVLink Server component and system ID #3592
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds support for configurable MAVLink system and component IDs in MAVLinkServer by introducing new attributes, initializing them from environment or defaults, and passing them as command-line flags. Class diagram for updated MAVLinkServer attributesclassDiagram
class MAVLinkServer {
log_path: Optional[str]
mavlink_system_id: Optional[int]
mavlink_component_id: Optional[int]
__init__()
_get_version() Optional[str]
name() str
}
MAVLinkServer --|> AbstractRouter
Flow diagram for MAVLinkServer initialization of system and component IDsflowchart TD
A["Start MAVLinkServer initialization"] --> B["Check if log_path is set"]
B --> C["Set log_path to default if not set"]
C --> D["Check if mavlink_system_id is set"]
D --> E["Set mavlink_system_id from environment or default (1)"]
E --> F["Check if mavlink_component_id is set"]
F --> G["Set mavlink_component_id to default (191)"]
G --> H["Pass IDs as command-line flags"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `core/services/ardupilot_manager/mavlink_proxy/MAVLinkServer.py:49-51` </location>
<code_context>
if not self.log_path:
self.log_path = "/var/logs/blueos/services/mavlink-server/"
+ if not self.mavlink_system_id:
+ self.mavlink_system_id = int(os.getenv('MAV_SYSTEM_ID', 1))
+ if not self.mavlink_component_id:
+ self.mavlink_component_id = 191
</code_context>
<issue_to_address>
**issue:** Environment variable parsing may fail if MAV_SYSTEM_ID is not an integer.
Add error handling for non-integer MAV_SYSTEM_ID values to prevent ValueError exceptions.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
if not self.mavlink_system_id: | ||
self.mavlink_system_id = int(os.getenv('MAV_SYSTEM_ID', 1)) | ||
if not self.mavlink_component_id: |
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.
issue: Environment variable parsing may fail if MAV_SYSTEM_ID is not an integer.
Add error handling for non-integer MAV_SYSTEM_ID values to prevent ValueError exceptions.
1833aa2
to
291deea
Compare
…component and system ID
291deea
to
41ac263
Compare
Thanks very much for this! |
Helps #3591
Summary by Sourcery
Add support for MAVLink system and component IDs in MAVLinkServer
Enhancements: