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

module: Websocket #1442

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

module: Websocket #1442

wants to merge 2 commits into from

Conversation

jheising
Copy link
Contributor

@jheising jheising commented Sep 1, 2024

Added a module to support input and output of MAVLink messages via a websocket connection.

I'm not sure what the process is for submitting a new module, but I figured I would create this PR to start the process. Questions:

  1. Do I need to create any tests? Wasn't clear to me if this was being done for other modules because I couldn't find any.
  2. Updates to MavProxy documentation? I assume I would submit this as an update the the ArduPilot MavProxy wiki if/when this PR was approved and merged?

Thanks!

Added a module to support input and output of MAVLink messages via a websocket connection.
@peterbarker
Copy link
Contributor

Don't we already have websocket support via Pet Hall's work in pymavlink here? ArduPilot/pymavlink#967

@jheising
Copy link
Contributor Author

jheising commented Sep 2, 2024

Well heck, @peterbarker I didn't notice that! Although it still might be worth a merge based on the following:

@IamPete1, I'm curious on your thoughts on potentially combining some of our efforts? I'm totally fine to close this PR and potentially use the core functionality in pymavlink, but would need some additional functionality for my needs.

These were the high-level requirements for my PR:

  1. Support for more than one concurrent connection.
  2. Support for both JSON and raw message formats.
  3. Support for API key validation for increased security in shared network environments.
  4. Support for both sending and receiving MAVLink messages.

From what I can tell:

  1. This PR supports more than one concurrent connection. Does the other one support more than 1 connection?
  2. This PR supports both JSON and raw MAVLink message formats. It looks like the other one only supports raw?
  3. This PR supports the ability to specify an API key to make the endpoint more secure. Other PR does not appear to support.
  4. This PR supports both inbound and outbound MAVLink messages. The other one says that client->server messages might work, but it doesn't appear to be tested and from a cursory glance at the code I'm not sure it will work (but could totally be wrong).

Anyway, I'm happy to abandon this PR if the other one can provide similar functionality. It's just a question of whether or not it would be easier to adapt this additional functionality into pymavlink or to add it to a module in MAVProxy. Also I guess the question is: does websocket support belong in a pymavlink connection_type or a MAVProxy module? I originally though about adding it as a connection_type, but when I saw that the REST interface was created as a module, I went the module direction. Curious what other people think.

Thanks!

@IamPete1
Copy link
Member

IamPete1 commented Sep 2, 2024

Hi @jheising. My implementation was down at the connection level because I don't want to have to run MAProxy, a simple translation script is enough. I have been using https://github.com/ArduPilot/pymavlink/blob/master/examples/mavtcpsniff.py with the second connection as websocket. What I have done is really a bare minimum any improvements would be great.

Some thoughts from my end:

  • JSON format requires that both ends of the WebSocket understand each message. The binary format requires only that the client end understands the message.
  • We could do both raw and json at once on using the text message and the other using the binary. However that will mean that half the messages we send are wasted.
  • If using the raw message multi client is harder as there is no guarantee that each client will send a complete message.
  • I would like to keep it simple enough that we could also add a websocket forwarding option to Mission Planner.

@jheising
Copy link
Contributor Author

jheising commented Sep 2, 2024

@IamPete1 to address some of the concerns:

JSON format requires that both ends of the WebSocket understand each message

Agreed, although the JSON format I used was simply the pymavlink.to_json function, so it should be as close to a "spec" as possible. The nice thing about supporting JSON is that you don't have to import a large client-side library in your code for parsing and serialization— you can let the server do it for you. For example, I can communicate directly to the JSON endpoint by just looking up the MAVLink documentation online and passing it a message with the exact same names as shown in the documentation:

{"mavpackettype": "COMMAND_LONG", "target_system":0, "target_component": 0, "command": 400, "confirmation": 0, "param1": 1, "param2": 0, "param3": 0, "param4": 0, "param5": 0, "param6": 0, "param7": 0}

So while there isn't a published JSON spec per se, the param names (except for mavpackettype) have a well-defined spec which makes it easy to translate into a common JSON format. It also makes it really easy to debug and test with command line tools like wscat or even the browser debugger console.

"We could do both raw and json at once on using the text message and the other using the binary"

There is no need to waste any bandwidth, you just need to indicate the format you want to communicate in when you connect. Since websockets start with a standard HTTP request we have full support for paths, URL parameters, etc., like any other HTTP request. So for example, if you want to send/receive raw/binary format you just connect to ws://myserver:myport and if you want to access the JSON format you just connect to ws://myserver:myport/json. The websocket server just looks at the URL path of the websocket connection to determine what format it should use.

"If using the raw message multi client is harder as there is no guarantee that each client will send a complete message."

Not sure how the websocket library you are using works, but the one I'm using treats each inbound connection as it's own instance, so I don't think there should be any issues with incomplete messages. Also the implementation I provided works in complete messages so it will only forward the messages to pymavlink when the message is complete.

Anyway, @IamPete1, totally fine if this version is a bit more overkill than what you needed and doesn't make sense to integrate into pymavlink.

@peterbarker I still think it would make sense to consider this merge into MAVProxy for the following reasons:

  1. For those of us wanting to create web-based interfaces into their autopilots, JSON is a logical and easy choice.
  2. JSON is partially supported in the REST module, but it appears to only support server->client, not client->server.
  3. JSON support via the REST module only works in a polling architecture, whereas this provides a real-time messaging architecture for better performance.

Please let me know what you think, thanks!

@jheising
Copy link
Contributor Author

jheising commented Sep 2, 2024

FYI, if you want to see a quick video of it in action: https://www.youtube.com/watch?v=5mEAPg7Ye7I

@IamPete1
Copy link
Member

IamPete1 commented Sep 5, 2024

FYI, if you want to see a quick video of it in action: https://www.youtube.com/watch?v=5mEAPg7Ye7I

Nice!

There is no need to waste any bandwidth, you just need to indicate the format you want to communicate in when you connect. Since websockets start with a standard HTTP request we have full support for paths, URL parameters, etc., like any other HTTP request. So for example, if you want to send/receive raw/binary format you just connect to ws://myserver:myport and if you want to access the JSON format you just connect to ws://myserver:myport/json. The websocket server just looks at the URL path of the websocket connection to determine what format it should use.

That is a nice solution, I had not thought of that. It would be great to also support it at the connection level.

message_name_2_type_cache = {}

# Some examples of JSON MAVLink messages
# {"mavpackettype": "COMMAND_LONG", "target_system":0, "target_component": 0, "command": 400, "confirmation": 0, "param1": 1, "param2": 0, "param3": 0, "param4": 0, "param5": 0, "param6": 0, "param7": 0}
Copy link
Member

Choose a reason for hiding this comment

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

You really want the header in here too. So you can check where the messages are coming from. There is already a "standard" for MAVLink over JSON from the mavlink2rest lib here: https://github.com/mavlink/mavlink2rest?tab=readme-ov-file#api

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great callout. I will update the PR to move to that.

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.

3 participants