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

Decode and use ESC_TELEMETRY_X_TO_Y mavlink packets as an AP_ESC_Telem_Backend #19668

Closed
wants to merge 2 commits into from
Closed

Decode and use ESC_TELEMETRY_X_TO_Y mavlink packets as an AP_ESC_Telem_Backend #19668

wants to merge 2 commits into from

Conversation

amilcarlucas
Copy link
Contributor

@amilcarlucas amilcarlucas commented Jan 3, 2022 •
edited

An alternative to #19666, using a completely different approach.

Binary Name      Text [B]        Data [B]     BSS (B)      Total Flash Change [B] (%)      Flash Free After PR (B)
---------------  --------------  -----------  -----------  ----------------------------  -------------------------
arducopter-heli  672 (+0.0388%)  0 (0.0000%)  0 (0.0000%)  672 (+0.0387%)                                   228792
blimp            672 (+0.0527%)  0 (0.0000%)  0 (0.0000%)  672 (+0.0527%)                                   689084
antennatracker   672 (+0.0500%)  0 (0.0000%)  0 (0.0000%)  672 (+0.0499%)                                   619868
ardusub          672 (+0.0431%)  0 (0.0000%)  0 (0.0000%)  672 (+0.0431%)                                   404548
arducopter       672 (+0.0383%)  0 (0.0000%)  0 (0.0000%)  672 (+0.0382%)                                   206400
arduplane        672 (+0.0382%)  0 (0.0000%)  0 (0.0000%)  672 (+0.0382%)                                   204612
ardurover        672 (+0.0425%)  0 (0.0000%)  0 (0.0000%)  672 (+0.0424%)                                   380668

andyp1per
andyp1per reviewed Jan 4, 2022
View reviewed changes
Copy link
Collaborator

@andyp1per andyp1per left a comment

Choose a reason for hiding this comment

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

Very cool. How do you switch it off?

@amilcarlucas
Copy link
Contributor Author

If no ESC_TELEMETRY_X_TO_Y mavlink packets come in, it will stop updating the AP_ESC_Telem.

Does that answer your question? Or Are you asking to use a preprocessor compile macro like "HAL_MAVLINK_ESC_TELEM_INPUT_ENABLED" ?

@tridge
Copy link
Contributor

tridge commented Jan 5, 2022

If no ESC_TELEMETRY_X_TO_Y mavlink packets come in, it will stop updating the AP_ESC_Telem.

seems a bit dangerous for something like two vehicles doing followme, or a swarm. They will each bounce packets back and forth between them

tridge
tridge requested changes Jan 5, 2022
View reviewed changes
Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

we could end up bouncing packets back and forth infiniitely

libraries/GCS_MAVLink/MAVLink_routing.cpp
@@ -122,6 +123,105 @@ bool MAVLink_routing::check_and_forward(mavlink_channel_t in_channel, const mavl
return true;
}

#if HAL_WITH_ESC_TELEM
Copy link
Contributor

Choose a reason for hiding this comment

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

this is really in the wrong place, should be in AP_ESC_Telem_Backend, and only when enabled

Copy link

Choose a reason for hiding this comment

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

Hi @tridge, Could you explain to me why these lines are in the wrong place? I have built this code and It works fine. I'm sorry. I'm a newbie for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

these messages are not targeted to a specific sysid, so I don't think they are safe to use in this way.
imagine 2 vehicles on the radio link, both generating these msgs, they would consume each others esc telem msgs and get the wrong RPMs to notch filter

@tridge tridge removed the DevCallEU label Jan 5, 2022
@amilcarlucas
Copy link
Contributor Author

How exaclly can I prevent the packets from bouncing?

tridge
tridge requested changes Feb 14, 2024
View reviewed changes
libraries/GCS_MAVLink/MAVLink_routing.cpp
@@ -122,6 +123,105 @@ bool MAVLink_routing::check_and_forward(mavlink_channel_t in_channel, const mavl
return true;
}

#if HAL_WITH_ESC_TELEM
Copy link
Contributor

Choose a reason for hiding this comment

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

these messages are not targeted to a specific sysid, so I don't think they are safe to use in this way.
imagine 2 vehicles on the radio link, both generating these msgs, they would consume each others esc telem msgs and get the wrong RPMs to notch filter

@tridge tridge removed the DevCallEU label Feb 14, 2024
@amilcarlucas
Copy link
Contributor Author

Thanks for the clarifications, closing it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andyp1per andyp1per andyp1per left review comments

@GithubKhang GithubKhang GithubKhang left review comments

@tridge tridge tridge requested changes

Assignees
No one assigned
Labels
AllVehicles Driver Enhancement NeedsHelp NeedsTesting WIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@amilcarlucas @tridge @andyp1per @GithubKhang