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 MAVProxy in testing of a custom MAVLink message #3390

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

wasaybaig
Copy link

I had a discussion with @hamishwillee that there is another easier way of testing your custom mavlink message. We can use MAVProxy which automatically uses messages in Pymavlink. However, if we clone the mavlink github repo, define our custom message, then generate the headers and then install pymavlink using setup.py in the mavlink repo, we can easily view our custom message on the MAVProxy console.

I find this method was easier and you could test it with MAVProxy without the need of building QGroundControl. Once this is tested on MAVProxy, you import the same headers in QGroundControl and build if required.

@wasaybaig wasaybaig marked this pull request as ready for review September 20, 2024 06:12
Copy link

github-actions bot commented Oct 2, 2024

No flaws found

@hamishwillee
Copy link
Collaborator

Thank you very much. I have restructured to put the update part in a separate section, and linking from where you had it. This makes the testing section a little more digestible. FWIW I prefer wireshark to this because it gives you traffic in both directions, but if you just want to test that the message is being sent, this is certainly pretty quick.

I think some of the instructions may not be quite right, but I'll add a separate comment.

1. Generate the headers:

```sh
python3 -m pymavlink.tools.mavgen --lang=C --wire-protocol=2.0 --output=generated message_definitions/v1.0/your_dialect.xml
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wasaybaig Are you sure you need this step?
This builds the C headers in mavlink/mavlink, not Python headers so I don't think this would be used.

Copy link
Author

Choose a reason for hiding this comment

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

@hamishwillee it works this way also but we can change the language to Python. But it is important to generate the headers with your new message or MAVProxy will not be able to understand the message.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But MAVProxy uses Python so this is a null operation - you haven't built anything that MAVProxy can use.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I'll have to try it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not needed. setup.py pulls any xml files in the parent definitions folder and builds them. Tested.


```sh
cd pymavlink
sudo python setup.py install
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not 100% certain this will work. Normally Pymavlink package is built with the ArduPilot fork of mavlink/mavlink headers.
What makes me think it probably does work is:

Copy link
Author

Choose a reason for hiding this comment

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

@hamishwillee yes I have tested it

Copy link
Contributor

Choose a reason for hiding this comment

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

Running this with sudo is asking to mess up your Python dependencies. I don't think that's something we should have in the docs.

What are we trying to do here? And why can't we just use upstream pymavlink?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this does indeed screw up your installation - as I have just verified. I can get this to "work" (somewhat) but I can't undo it. Will have to delete my installation and start again :-(

What we are trying to do, ultimately, is have an easy way to test a custom message, since building QGC with a custom dialect is a bit of a pain.

The solution described here is to install mavproxy and then delete the pymavlink that it depends on. Then rebuild pymavlink from the mavlink repo clone with your new xml definitions added. This step is easy because you just copy the xml files in and run setup.py and it creates a new installation/.egg with the .py files for the definitions.
Mavproxy can then display the message if it is received.

  1. We definitely don't need the step above building the files from the definitions. the setup.py does that for you.
  2. This still doesn't work though due to the error MAV_TYPE_VTOL_DUOROTOR not defined. That is correct - it has been removed from Mavlink, but it seems that mavproxy expects it in https://github.com/ArduPilot/MAVProxy/blob/master/MAVProxy/modules/mavproxy_link.py#L744
    • @wasaybaig How did you work your way around that? It's a bug in mavproxy.
  3. I presume we can completely avoid all use of sudo if we install mavproxy and run setup from inside a virtual env?

Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, let's see if we can get MAVProxy to update ArduPilot/MAVProxy#1470

@peterbarker
Copy link

Just thought I'd mention my workflow for using modified message definitions.

pbarker@fx:~/rc/pymavlink(master)$ MDEF=~/rc/ardupilot/modules/mavlink/message_definitions pip install .

Running setup.py is deprecated and is likely to break in the next few years - I'm retraining my brain, I suggest other people start to do likewise :-)

@hamishwillee
Copy link
Collaborator

Thanks @peterbarker ! I'll add that to the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants