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.

Sign up for GitHub

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

Jump to bottom

Enable setting motor power level through MSP #1214

Merged
merged 2 commits into from
Feb 20, 2023
Merged

Enable setting motor power level through MSP #1214

merged 2 commits into from
Feb 20, 2023

Conversation

victorhook
Copy link
Contributor

This PR adds functionality to set the motor power level through MSP, which can be done directly from ESC Configurator for instance (using the motor sliders). This can be very handy when configuring the Crazyflie Bolt.
I also refactored some code in msp.c.

A concern I discovered when working with the Crazyflie Bolt and ESC Configurator:

  1. Sometimes the ESC flashing procedure through ESC Configurator fails before it's finished. The Crazyflie crashes with watchdog timeout, which results in a broken ESC which has to be un-bricked manually (for instance with C2 interface). I assume that there is something in esc4wayProcess() function that takes too long, but have not investigated this further.

@victorhook
Added support to set motor power through MSP (for instance with ESC C…
e73dddf 
…onfigurator)

- Two public functions were added to motors.h and motors.c to enable this.
- msp.c was also extended to support command MSP_SET_MOTOR
- Fixed the msp request state machine to support data in requests, which was not supported prior to this.
- Fixed function mspIsRequestValid() to no longer discard packets with payload data, since certain requests (like MSP_SET_MOTOR) DOES contain data in the packet.
- Refactored old functions to use helper-function mspMakeTxPacket() instead of manually assembling packets every time.
@tobbeanton
Copy link
Member

Nice addition, something we should probably have in the cfclient as well.
What I'm thinking about is how to best change parameters from within the FW. In this example it is not really a problem since the 4way task hogs the whole system but one might imagine there could be a discrepancy between what e.g. the cfclient would know and what the FW is set to. Thus when a parameter is changed this should be notified to connected clients. Let us discuss internally and get back to you.

@victorhook
Copy link
Contributor Author

That's a good point. Wasn't sure if and how you wanted the public API (to motors.h) to enable and set power level of the motors so I did something that perhaps can serve as base.

@knmcguire knmcguire requested a review from tobbeanton February 8, 2023 08:29
@ataffanel
Copy link
Member

The best would be to set parameters using the internal param API. This will send updated value to a potentially connected client so it make sure everything stays in sync.

@victorhook
Changed the power-set functionality to use the internal param API ins…
d9227bc 
…tead of the public functions in motors.h.

Note that this requires the Crazyflie to be connected to the cfclient when setting motor power, OR the build flag CONFIG_PARAM_SILENT_UPDATES must be set.
@victorhook victorhook force-pushed the esc-configurator-motor-control branch from bed4f73 to d9227bc Compare February 14, 2023 21:19
@victorhook
Copy link
Contributor Author

Good idea Arnaud, I made a new commit where I removed the newly added public functions in motors.h and instead used the internal parameter API to enable and set the motor power values.
However, it's worth mentioning that using the internal param API instead requires the Crazyflie to be connected to the cfclient, or the flag CONFIG_PARAM_SILENT_UPDATES must be set, as described here.

I've tried this with cf bolt, using ESC Configurator and both options:

  1. Ensure Crazyflie is connected to client.
  2. Set CONFIG_PARAM_SILENT_UPDATES build flag.

works :)

@krichardsson
Copy link
Contributor

It i s a bit of a problem that it does not work unless a client is connected. I added an issue for this #1234.

@krichardsson krichardsson merged commit ee32547 into bitcraze:master Feb 20, 2023
@krichardsson
Copy link
Contributor

Thanks!

@krichardsson krichardsson added this to the 2023.02 milestone Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobbeanton tobbeanton Awaiting requested review from tobbeanton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2023.02
Development

Successfully merging this pull request may close these issues.
4 participants
@victorhook @tobbeanton @ataffanel @krichardsson