Skip to content

Conversation

@AnasChebili
Copy link

This PR adds the implementation of the get_vehicle_state function.

I tested the full execution flow (with MP connected to SITL), and everything works as expected. I've attached a video showing it.

Also, function outputs are now converted into JSON-formatted strings, so functions can return objects and arrays.

Screen.Recording.2025-03-25.163012.mp4

@codemaster1104
Copy link

codemaster1104 commented Mar 26, 2025

Hey @AnasChebili
I just tested your code and it looks like mavlink message handling isn't working correctly and for vehicle state it sometimes return 0 value by default, even I faced same in starting as shown below.
Screenshot from 2025-03-16 00-19-24
In picture above even my message handling was not working correctly but it still returns those value by default.

After correct implementation it should look something like this
Screencast from 03-26-2025 10:54:11 PM.webm
This video is of my implementation of functions

@AnasChebili
Copy link
Author

hey @codemaster1104,
I think there’s some confusion.
To clarify, here is a more thorough test for your reference:

test-recording.mp4

Also, I checked out your PR (#5), and when I tried connecting to MP, I ran into this error:

aperror

After reviewing your code, I see where the confusion might be. It looks like you’ve set up a separate WebSocket server and are connecting to it, routing to SITL through it. However, our approach is to connect to the vehicle through MP, which already runs a WebSocket process on port 56781 by default (This is why the port number is hard-coded, it wasn’t an oversight. I noticed you changed it in your PR, but I think we should keep it as it is).

From what I can tell in PR #5, it seems you’re sending vehicle data from your server as JSON. That approach would indeed not work with the message handling solution implemented by @rmackay9, since MP sends data as an array buffer rather than JSON. Given that we’re connecting through MP, the current solution works fine, this also eliminates the need for the additional helper functions you added.

I hope this explanation helps clarify things! Also, thank you for the review!

@codemaster1104
Copy link

codemaster1104 commented Mar 27, 2025

Hey @AnasChebili , thanks for pointing it out it is indeed working with Mission Planner , actually I was working with QGC(as project idea also states it) and it doesn't seem to start its own websocket thats why i needed to implement my own websocket functionality which can be condensed to a singular script file, and the issue you incurred on my PR is most probably because I am expecting data as a json string and MP is sending it as different datatype.
@rmackay9 Do I keep implementing more on my approach for QGC/direct SITL HITL as it bypasses the need of MP or QGC?

@AnasChebili
Copy link
Author

hey @codemaster1104,
You're right that the project idea mentions both MP and QGC as connection options. However, I initially interpreted it as an exclusive choice between the two. After reviewing the current implementation, I assumed that MP was the chosen approach, though that could just be my assumption.

Regarding the message handling issue, this shouldn't be too difficult to address. MP sends the message data as an array buffer (not a string), whereas you're sending it as JSON. Wouldn't it be more consistent to send the same data type as MP? That way, if we later decide to support both MP and QGC, we can maintain a consistent message format and avoid implementing separate handling logic for different cases.

@rmackay9
Copy link
Owner

I've resolved the merge commit and pushed, thanks again!

@rmackay9 rmackay9 closed this Apr 28, 2025
@AnasChebili AnasChebili deleted the chat-test-state branch June 11, 2025 20:33
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