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

Feat/low speed data transmission #58

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

Conversation

Yamboy1
Copy link

@Yamboy1 Yamboy1 commented Feb 15, 2024


Basic Info

Info Please fill out this column
Ticket(s) this addresses N/A (but see #39)
Primary OS tested on Ubuntu
Drone platform tested on Matrice 350 RTK on PSDK 3.8

Description of contribution in a few bullet points

Adds a new module for communication via the low speed data channel from an MSDK based app to the drone. Received information is published by the "psdk_ros2/low_speed_transmission_data" topic as a string.

Also adds .vscode to the gitignore, and removes a couple of instances of trailing whitespace.

Motivation and Context

We would like to have communication between the drone and the RC remote via the PSDK apis, as in the past we have only been using rosbridge (ROS1). See my other issue regarding high speed data channel: #39. We were unable to get high speed transmission working, but we have been able to get the low speed transmission working, so we're making this PR to upstream it.

For context see:

How Has This Been Tested?

We have tested the changes on a Matrice 350 RTK, connected to a DJI RC Plus. The onboard computer is an Intel NUC.

On the nuc, we ran ros2 launch psdk_wrapper wrapper.launch.py to start the wrapper. In a second terminal, we had ros2 topic echo /wrapper/psdk_ros2/low_speed_transmission_data running to view the data.

On the RC, we had the MSDK sample application installed (https://github.com/dji-sdk/Mobile-SDK-Android-V5). In the app we selected "Testing Tools > PSDK Test > Open Payload Data Page > External". We then entered text into the field and on pressing send, the text should appear in the rostopic echo command running on the NUC.

Additionally, we also had to comment out init_camera_manager(), init_gimbal_manager() and init_liveview() from https://github.com/Yamboy1/psdk_ros2/blob/afd4ff1061fb8b29cd269ceab7b6588d7175925f/psdk_wrapper/src/psdk_wrapper.cpp#L85, as we do not currently have a gimbal or camera plugged into a payload port, but this should not make a difference in this case.

Description of documentation updates required from your changes

We've just added a new publisher, so this should be the only documentation that needs to be added.

Future work that may be required in bullet points

There might be some work required to add support for non text (char*) encoded data sent over the channel.
Additionally there's a small hack (with a comment) that seemed necessary to get around an extra byte at the start of the data. I may have gone about fixing that the wrong way so please have a look at it.

Screenshots (if appropriate):

@Yamboy1 Yamboy1 requested a review from a team as a code owner February 15, 2024 21:38
@Yamboy1
Copy link
Author

Yamboy1 commented Feb 15, 2024

I haven't had a chance to compile and test the new changes since the original PR while I'm out of the office for the week.

@vicmassy
Copy link
Contributor

please if you want us to take a look, ensure there are no conflicts before asking for review

@Yamboy1
Copy link
Author

Yamboy1 commented Mar 4, 2024

Sorry about that @vicmassy, conflicts should be sorted now.

@yanxke
Copy link

yanxke commented Jun 17, 2024

@Yamboy1 @biancabnd Is it possible to merge this in? I'm interested in this feature as well. Thanks!

@Yamboy1
Copy link
Author

Yamboy1 commented Jun 21, 2024

I believe I've done all I need to, just waiting on a response from the developers

@yanxke
Copy link

yanxke commented Jun 21, 2024

I believe there are still merge conflicts to be resolved.

@Yamboy1
Copy link
Author

Yamboy1 commented Jun 21, 2024

Oh huh, I thought I had fixed those, I'll have a look tomorrow, thanks @yanxke

@Yamboy1
Copy link
Author

Yamboy1 commented Jun 24, 2024

So it turns out there have been a lot of changes since my original pull request, so this might take me a bit more time to sort out than I'd originally expected

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