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

Handling of Commander Packets #24

Open
whoenig opened this issue Jan 26, 2022 · 4 comments
Open

Handling of Commander Packets #24

whoenig opened this issue Jan 26, 2022 · 4 comments
Labels
discussion enhancement New feature or request

Comments

@whoenig
Copy link
Contributor

whoenig commented Jan 26, 2022

This is a discussion to decide on how to properly handle packets on ports CRTP_PORT_SETPOINT and CRTP_PORT_SETPOINT_GENERIC. Messages to those ports are real-time commands (such es desired roll/pitch angles). As such, they should not be repeated even if they don't make it to the CF. Ideally, we would also only keep a queue of size 1, i.e., we should never send "old" commands, just in order to flush out our queue. Options:

  1. Add a new single-CF address that works similar to the current broadcasts. This requires a (NRF) firmware change, a small change here, and minor changes for users of crazyflie-link-cpp. We could also add an option in this library to keep the queue size to 1 and to give all broadcasts highest priority.
  2. Add a flag in the Packet data structure to disable retry, i.e., we don't need any firmware changes. The downside is that we still use safelink in that case, which could reject a packet of host and CF are out-of-sync. To keep the queue size to 1, we would need special handling for the two ports mentioned above.

Test case: a client application that sends setpoints at a very high rate (e.g., 1 kHz), should still be able to fly the Crazyflie well.

@ataffanel: It would be nice to get your thoughts on this.

@knmcguire
Copy link
Member

hi! Arnaud will be gone for a while so I'm looking through the isues from the repo. I still need to get more familiar with the code.

It is interesting though. So just to send setpoints to the crazyflie without expecting an ack. So a broadcast with an CF2 address attached.

But I guess we still need to have the occasional ping from the crazyflie in question, just to check if it is still receiving or not? I guess this ping needs to be a seperate package.

@knmcguire knmcguire added discussion enhancement New feature or request labels Mar 9, 2022
@whoenig
Copy link
Contributor Author

whoenig commented Mar 22, 2022

Yep, exactly! It is true that the ping is still needed. This is currently done automatically in this library, whenever no other packet is in the queue.

@whoenig
Copy link
Contributor Author

whoenig commented May 30, 2022

I accidentally pushed a change for option 2 already in 1f9d893 without a PR/review. Sorry about that. Post-review would be welcome, or I can revert the change and open a proper PR.

@knmcguire
Copy link
Member

Ah woops 😄

that is okay, it seemed to be all fine. So let's keep it like this for now.

The only thing is that if we want to do a release, the github automatic changes list will not note this in particular, so that why PRs are usually better, just to let you know for next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants