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.

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

Added transponder plugin #1390

Merged
merged 18 commits into from
Apr 6, 2021
Merged

Added transponder plugin #1390

merged 18 commits into from
Apr 6, 2021

Conversation

DoppleGangster
Copy link
Contributor

@DoppleGangster DoppleGangster commented Mar 31, 2021

Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

Good start! Do you have a way to test that the conversions are correct? Like do you have a mavlink component that emits ADS-B messages?

src/plugins/transponder/transponder_impl.cpp Outdated Show resolved Hide resolved
src/plugins/transponder/transponder_impl.cpp Outdated Show resolved Hide resolved
src/plugins/transponder/transponder_impl.cpp Outdated Show resolved Hide resolved
src/plugins/transponder/transponder_impl.cpp Outdated Show resolved Hide resolved
@JonasVautherin
Copy link
Collaborator

One more thing: you need to fix the style, we have a script for that: ./tools/fix_style . should do it 👍

DoppleGangster and others added 4 commits March 31, 2021 13:07
Co-authored-by: Jonas Vautherin <jonas.vautherin@protonmail.ch>
Co-authored-by: Jonas Vautherin <jonas.vautherin@protonmail.ch>
Co-authored-by: Jonas Vautherin <jonas.vautherin@protonmail.ch>
Co-authored-by: Jonas Vautherin <jonas.vautherin@protonmail.ch>
@DoppleGangster
Copy link
Contributor Author

I thought I had already run the style script. I'll make sure to run it. I have an ADSB module ordered and it suppose to be delivered today.

@DoppleGangster
Copy link
Contributor Author

Style should be correct now.

Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

Somehow all the conversions got lost in the process (i.e. all my suggestions). Did you revert them? 🤔

@DoppleGangster
Copy link
Contributor Author

I uploaded the files with the style changes. Not sure if that removes comments?

@DoppleGangster
Copy link
Contributor Author

I think I know what happened. I'll go through and reimplement your recommendations tomorrow.
Sorry about that.

@JonasVautherin
Copy link
Collaborator

Also you get some unrelated CI issues, please rebase once #1392 is merged and that should solve them 👍

@DoppleGangster
Copy link
Contributor Author

@JonasVautherin: I've re-implemented your commits that were lost.

@JonasVautherin
Copy link
Collaborator

Starting to look good! Should we now wait until you can actually test it, and then move forward to merge it?

@DoppleGangster
Copy link
Contributor Author

Sounds good to me. My expected receive date has gone to pending so most likely won't have the test results until sometime next week. Thanks for you help with this!

Comment on lines 32 to 33
_parent->unregister_statustext_handler(this);
_parent->unregister_param_changed_handler(this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are those needed? They are not registered in the init, are they? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh yeah you're right. I'll remove them. I was using the telemetry_imp.cpp as a guide and might have gotten carried away with what I was bring over. I've received the sensor however my test code seems to have issues. My subscription that I create should output text at 1 Hz however I never get any output from the function.

Is there another tie in somewhere that I need to get mavsdk to register my subscription?

Created Example

@DoppleGangster
Copy link
Contributor Author

@JonasVautherin: If I register for the subscription updates in the test program for the transponder API should I expect to get the values reported even if the transponder if not communicating? I get no errors when I set the update rate and subscription but it does not appear that the callback is ever executed.

@JonasVautherin
Copy link
Collaborator

If I register for the subscription updates in the test program for the transponder API should I expect to get the values reported even if the transponder if not communicating?

When you subscribe, you will receive an update everytime the MAVLink message is sent. If your transponder does not send the MAVLink message, then nothing will happen. I'm expecting that the receiver is onboard the drone, receives ADS-B data and sends them as MAVLink messages on the sysid of the drone. Is that correct?

@DoppleGangster
Copy link
Contributor Author

The receiver is onboard and I configured according to the PX4 Air Traffic Setup. I'm assuming this would be the same configuration to be able to access the MAVLINK messages for the ADS-B. The only difference in hardware is I'm using the NXP-FMU with a PingRX versus the mRO Pixhawk mentioned in the page.

@DoppleGangster
Copy link
Contributor Author

@JonasVautherin : It does not appear that I have the ability to change the ADS-B module to use the drones sysid. Is there a way to have MAVSDK report messages of a certain type regardless of the sysid or do I need to find a way to grab the broadcast messages from the telemetry port that is communicating with offboard program?

@TSC21
Copy link
Member

TSC21 commented Apr 5, 2021

@DoppleGangster did you check if the firmware at runtime enters this loop: https://github.com/PX4/PX4-Autopilot/blob/master/src/modules/mavlink/streams/ADSB_VEHICLE.hpp#L69. Have you tried accessing the nsh shell (can be done through the MAVLink shell in QGC) and type listener transponder_report? If it states something like never published then something is wrong in your setup.
Before debugging MAVSDK, the first thing you need to do is making sure you actually have the transponder data being generated and propagated in PX4 and also being streamed through MAVLink.

@DoppleGangster
Copy link
Contributor Author

@TSC21 thanks for the comment. I confirmed that the data was never published. I've corrected the setup and now I am getting data from the transponder over MAVSDK.

@JonasVautherin and @TSC21 thank you both for your help with debugging this.

@DoppleGangster
Copy link
Contributor Author

The plugin seems to be fully operation from my testing. Assuming no more changes are requested should be ready to merge with the proto fork update I would think?

@TSC21
Copy link
Member

TSC21 commented Apr 5, 2021

The plugin seems to be fully operation from my testing. Assuming no more changes are requested should be ready to merge with the proto fork update I would think?

@DoppleGangster for both being ready for merge, you need to first squash the commits.

Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

Good to read that it works!

I merged the MAVSDK-Proto PR. @DoppleGangster now you need to update the proto submodule such that it points to main on mavsdk-proto, and fix the style in this PR. Then we can merge it 👍 🚀

@DoppleGangster
Copy link
Contributor Author

Proto module update is in and style updated. Thanks again!

@JonasVautherin JonasVautherin merged commit 3770ef6 into mavlink:main Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants