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

Add DShot module #12854

Merged
merged 27 commits into from
Oct 11, 2019
Merged

Add DShot module #12854

merged 27 commits into from
Oct 11, 2019

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented Aug 31, 2019

This brings #12634 from @Igor-Misic and adds a new output module that uses the low-level DShot interface. The PR includes the commits from #12790.

The dshot module is implemented as drop-in replacement for fmu, which means it handles the same modes so that other functionality is not lost - the duplication is however not nice.

I've flown this on the Omnibus BetaFPV tiny whoop with DShot600.

DShot is enabled on OmnibusF4SD and Pixracer.
Caution: Before flying this on a Pixracer, we need to double-check that there are no DMA resource conflicts.

Missing/next steps:

  • Support for F7 (including DMA-able memory and flushing caches). @Igor-Misic is that something you're still interested in/ already looking into?
  • Enable on all other boards.
  • More testing, also on boards with IO.
  • DShot Telemetry

@Igor-Misic
Copy link
Member

@bkueng, first congrats on this amazing work you have done so far, and for the first DShot PX4 flight :)

I went through F7 DMA drivers (Nuttx DMA for F7) and I think this may work just as it is. F4 and F7 share the same registers for DMA. As you helped me on Slack to understand FMUv5 boards I added defines to skip that unsupported IO Processor (F1 MCUs) building Dshot for them. Firmware is building for F7 boards, and it should use their own drivers (nuttx/arch/arm/src/stm32f7/smt32_dma.h) for DShot drivers.
Unfortunately, I don't have any F7 board to run and check it.

@dagar
Copy link
Member

dagar commented Aug 31, 2019

The dshot module is implemented as drop-in replacement for fmu, which means it handles the same modes so that other functionality is not lost - the duplication is however not nice.

One idea I was going to explore was continue splitting all the px4fmu functionality into separate minimal modules and rely on the underlying channel allocations in io_timer to prevent conflicts. Pin ownership would then become first come, first served.

drivers/px4fmu split into:
      -> drivers/pwm_out
      -> drivers/input_capture
      -> i2c and peripheral reset -> drivers/boardctl (or almost anywhere else...)

Instead of modes the fmu module (now simply pwm) would try to take the required pins when loading a mixer. If there's a conflict we can tell the user exactly what's happening (trying to load a mixer with 6 outputs and pins 5 & 6 already taken for gpio, etc) and either completely abort, or only use a subset.

Pros

Cons

  • likely harder to convey the overall pin configuration to a user for configuration in QGC than if we went for a single unified px4fmu
  • additional constraints on timer boundaries we might need to respect

@hamishwillee
Copy link
Contributor

@bkueng Can you please ping me when Dshot is in a documentable form - I presume this is not going into PX4 v1.10?

In the docs we we explicitly state that we don't support it in the docs - so we would need to remove that. If there is any explicit config or different hardware set up we'd want to capture that too.

@bkueng bkueng force-pushed the fmu_refactoring_output_module__dshot branch from ce17e99 to 23003e9 Compare September 5, 2019 14:01
@bkueng
Copy link
Member Author

bkueng commented Sep 5, 2019

@bkueng, first congrats on this amazing work you have done so far, and for the first DShot PX4 flight :)

Thanks! The larger portion of the credits goes to you, without the lower level parts it would not be possible.

I went through F7 DMA drivers (Nuttx DMA for F7) and I think this may work just as it is.

Indeed, it does! I expected more work would be needed.


@dagar yes that might work.


@hamishwillee sure. The short answer: as soon as this goes in it's usable (we need to enable it on all boards though).

Updates

  • added F7, enabled KakuteF7
  • added (indirect) telemetry via UART and publish esc_status
  • added CLI commands, for example:
    • let the ESC beep: dshot beep1 -m 1
    • permanently reverse motor direction (no more ESC resoldering :)
    dshot reverse -m 1
    dshot save -m 1
    
    • retrieve ESC infos: dshot esc_info -m 2:
nsh> dshot esc_info -m 2
INFO  [dshot] ESC Type: #TEKKO32_4in1#
INFO  [dshot] MCU Serial Number: 480022-001751-564e34-323420
INFO  [dshot] Firmware version: 32.60
INFO  [dshot] Rotation Direction: normal
INFO  [dshot] 3D Mode: off
INFO  [dshot] Low voltage Limit: off
INFO  [dshot] Current Limit: off
INFO  [dshot] LED 0: unsupported
INFO  [dshot] LED 1: unsupported
INFO  [dshot] LED 2: unsupported
INFO  [dshot] LED 3: unsupported

Known Issues

Problems I came across while testing:

  • Sometimes (quite rare) upon disarming the ESCs re-initialize, so there is something about the signal they don't like. I see this on 2 completely different quads (F7 vs F4 and different ESCs), so it's unlikely a HW-problem.
  • Telemetry and sending of commands is not 100% reliable. Sometimes a command is not executed, and telemetry requests regularly and randomly time out. A cross-test with betaflight sending commands on the same HW does not show that issue.

@hamishwillee
Copy link
Contributor

@hamishwillee sure. The short answer: as soon as this goes in it's usable (we need to enable it on all boards though).

And the long answer? :-) Because I take that to mean that these instructions will work without any change http://docs.px4.io/master/en/peripherals/pwm_escs_and_servo.html

@bkueng
Copy link
Member Author

bkueng commented Sep 17, 2019

This can be tested on Pixracer now, e.g. by setting DSHOT_CONFIG to 1200.

@mhkabir
Copy link
Member

mhkabir commented Sep 23, 2019

@bkueng How can we get this working on Pixhawk 4?

@bkueng bkueng force-pushed the fmu_refactoring_output_module__dshot branch from ea90eae to 4ffbe0c Compare September 24, 2019 17:58
@bkueng
Copy link
Member Author

bkueng commented Sep 24, 2019

@bkueng How can we get this working on Pixhawk 4?

I enabled it on v5, but only for the first 4 outputs (the next ones conflict with px4io
serial DMA (UART8_RX)). I had to disable RX DMA on the GPS port - can you test GPS as well?

@mhkabir
Copy link
Member

mhkabir commented Sep 24, 2019

OK, let me have a look , thanks :)

@bkueng bkueng force-pushed the fmu_refactoring_output_module__dshot branch 5 times, most recently from 049b662 to b8710af Compare September 27, 2019 06:02
@bkueng
Copy link
Member Author

bkueng commented Sep 27, 2019

I've flown this quite a bit on Pixracer and KakuteF7 (Kopis 2 frame with telemetry), so that I'm confident it's working.

Flight Performance

On both vehicles I've flown (250 size), I noticed considerably smoother flight performance - both audibly and visibly. Prop-wash handling is noticeably improved.
Part of this is certainly due to the 1kHz rate loop, and I have not (yet) checked how much of it is to attribute to DShot.

Boards

DShot is enabled on these boards:

  • Pixhawk (v3): enabled on all outputs, disabled RX DMA on TEL4 and IO debug port
  • Pixhawk 4 (v5): enabled only on first 4 outputs, disabled RX DMA on GPS port
  • Pixracer (v4): enabled on all outputs, disabled RX DMA on FrSky telem port
  • OmnibusF4SD: enabled on all outputs, disabled RX DMA on USART1
  • KakuteF7: enabled on all outputs, disabled RX DMA on UART4 (GPS port)

Issues

Issues from above:

Sometimes (quite rare) upon disarming the ESCs re-initialize, so there is something about the signal they don't like. I see this on 2 completely different quads (F7 vs F4 and different ESCs), so it's unlikely a HW-problem.

The problem is the flash-based param saving: in some cases the entire CPU stalls for ~300ms, which causes DShot not to update anymore (on PWM and Oneshot, the timers continue to trigger). If in-air, the motors might stop, so the param auto-saving is now deferred until disarming for flash-based param storage.

Telemetry and sending of commands is not 100% reliable. Sometimes a command is not executed, and telemetry requests regularly and randomly time out. A cross-test with betaflight sending commands on the same HW does not show that issue.

Fixed by updating the timer config before triggering the DMA update.

Logs

With a Pixracer:

Other

  • CPU load is about 1% higher and ~1KB more RAM is used with DShot vs PWM (RAM is due to the DMA buffers)
  • If for some reason the DShot signal stops updating while motors are spinning (e.g. actuator topic is not updated), the motors slowly stop and disarm.
  • There is a pending refactoring from @Igor-Misic to use NuttX DMA methods, but it still has issues, so we can merge that separately.
  • Once we use ITCM RAM on F7, we'll need to use DMA memory allocators for the DShot buffers.

@PX4/testflights can you test this on Pixhawk (v3), Pixracer and Pixhawk 4 on vehicles with ESCs that support DShot? You can follow the instructions here: https://github.com/PX4/px4_user_guide/pull/578/files#diff-58ea316b3b439c16b17ab4f924adc37a. Be careful, the vehicle might need some re-tuning.

I'm not sure if we want to pull this into v1.10?

@dagar
Copy link
Member

dagar commented Sep 27, 2019

Once we use ITCM RAM on F7, we'll need to use DMA memory allocators for the DShot buffers.

We're currently using non-DMA capable memory in the heap on F7.

https://github.com/PX4/Firmware/blob/4f71c4e123a627ce0724b8065089c8e86038aebc/boards/px4/fmu-v5/nuttx-config/scripts/script.ld#L76

On both vehicles I've flown (250 size), I noticed considerably smoother flight performance - both audibly and visibly. Prop-wash handling is noticeably improved.
Part of this is certainly due to the 1kHz rate loop, and I have not (yet) checked how much of it is to attribute to DShot.

It would be interesting to compare a few flights at different loop rates. You can do this now with the IMU_GYRO_RATEMAX parameter. With a bit more work (drivers cleanup + drdy, standalone rate controller, optimization in a few key places) we should be able to get to 2 kHz "soon".
Related - betaflight/betaflight#7327 (comment)

And handle failures of up_dshot_init().

On Omnibus: reduces memory usage if dshot is enabled by ~1.0KB.
The buffer is roughly 1KB in size.
No other functional change, just restructuring.
This reloads the timer configuration before triggering DMA. Without that,
in rare cases, there were 17 bits sent instead of 16.
The 1. bit (1. pulse) was always wrong (too much), the rest of the bits
were the correct DShot packet that was meant to be sent.
But only on the first 4 FMU outputs, as the next ones conflict with px4io
serial dma (UART8_RX)

RX DMA is disabled on the GPS port as well (conflicts with TIM1).
This is especially important for DShot, that does not update when the CPU
stalls.
Disables RX DMA on TEL4 and IO debug serial port
@bkueng bkueng force-pushed the fmu_refactoring_output_module__dshot branch from 0a96148 to 69a5561 Compare October 10, 2019 09:57
@bkueng bkueng merged commit 803a719 into master Oct 11, 2019
@bkueng bkueng deleted the fmu_refactoring_output_module__dshot branch October 11, 2019 06:14
@MaEtUgR
Copy link
Member

MaEtUgR commented Oct 11, 2019

This is so awesome!!! 🚀

I come from flying the Holybro Kopis 2 with the new DSHOT support and it flies that good now! It's really confidence-inspiring.
Next time I'll bring a GoPro but here's the eye cancer stream such that you get an impression:
https://youtu.be/XDT9YaBVlEs
Here are the two flight logs:
https://logs.px4.io/plot_app?log=786298ac-1b19-4019-be56-130c64a1baf0
https://logs.px4.io/plot_app?log=4c7c0414-e84e-47d6-9116-6fda9c0cb67e

Thanks a lot to @bkueng @dagar @Igor-Misic and everyone else who helped for doing all these low-level improvements that will drastically improve any low latency actuated vehicle's flight performance!

@thomasderflieger
Copy link

Since being spoiled from flying racing-quadrocopters with DSHOT and telemetry for quite some time i am very happy to see those features being implemented into PX4. Thank you!

I'm currently building a VTOL with the CUAV V5 Nano Flight Controller. It has 3 Motors and 5 Servos, wich is also exactly the number of PWM outputs of the FC. It is similar to the Convergence-airframe, but i can also tilt the rear Motor.

I enabled it on v5, but only for the first 4 outputs (the next ones conflict with px4io
serial DMA (UART8_RX)). I had to disable RX DMA on the GPS port - can you test GPS as well?

Doas that mean if i enable DSHOT the first 4 outputs are going to use DSHOT or doas it depend on the motor-count. If it is enabled on the first 4 outputs, i could not use nr. 4, but then im missing an PWM output for one of my Servos.

The second question is regarding logging. Is it possible to log the ESC telemetry data and add it to the logfiles? That could help with analysis a lot.

@hamishwillee
Copy link
Contributor

hamishwillee commented Oct 15, 2019

@thomasderflieger Docs are here: http://docs.px4.io/master/en/peripherals/dshot.html. You wire according to your mixer file/airframe reference: http://docs.px4.io/master/en/airframes/airframe_reference.html. Ie it is enabled for whatever motors you have connected, but not for servos.

@bkueng Would need to comment on logging.

@bkueng
Copy link
Member Author

bkueng commented Oct 16, 2019

Doas that mean if i enable DSHOT the first 4 outputs are going to use DSHOT or doas it depend on the motor-count. If it is enabled on the first 4 outputs, i could not use nr. 4, but then im missing an PWM output for one of my Servos.

It's indepenent on the motor count and the current implementation does not allow for mixing DShot with PWM outputs on the FMU pins (on a board with an IO you could use PWM on the IO pins and DShot on the FMU pins).

The second question is regarding logging. Is it possible to log the ESC telemetry data and add it to the logfiles? That could help with analysis a lot.

It already is. It is the esc_status message.

@thomasderflieger
Copy link

It's indepenent on the motor count and the current implementation does not allow for mixing DShot with PWM outputs on the FMU pins (on a board with an IO you could use PWM on the IO pins and DShot on the FMU pins).

Thats a pitty, was looking forward to using dshot for my VTOL, my current flight controller just has main-ports though. I assume that the Telemetry wont work without using dshot either, since the ESC's need a "telemetry-request message"?

It already is. It is the esc_status message.

Great, thanks!

@bkueng
Copy link
Member Author

bkueng commented Oct 16, 2019

Thats a pitty, was looking forward to using dshot for my VTOL, my current flight controller just has main-ports though. I assume that the Telemetry wont work without using dshot either, since the ESC's need a "telemetry-request message"?

Exactly. Yeah, that's a drawback, but with all configuration options and timer restrictions, it would become really complex.

@ndepal
Copy link
Contributor

ndepal commented Oct 16, 2019

It's indepenent on the motor count and the current implementation does not allow for mixing DShot with PWM outputs on the FMU pins (on a board with an IO you could use PWM on the IO pins and DShot on the FMU pins).

Just do be sure, does that mean that it's not possible to use camera triggering (GPIO) on a PixRacer together with DShot?

@bkueng
Copy link
Member Author

bkueng commented Oct 17, 2019

Just do be sure, does that mean that it's not possible to use camera triggering (GPIO) on a PixRacer together with DShot?

That is possible. Do you have a setup where you can test it?

@ndepal
Copy link
Contributor

ndepal commented Oct 17, 2019

yes we could probably test that pretty easily

}

// publish when motor index wraps (which is robust against motor timeouts)
if (motor_index < _telemetry->last_motor_index) {
Copy link
Contributor

@ndepal ndepal Oct 17, 2019

Choose a reason for hiding this comment

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

When we were testing dshot with just one ESC connected we could not get telemetry info on esc_status because of this line. I don't know if 1 ESC is an actual use case (planes?), but if it is, would the following be better?

if (motor_index <= _telemetry->last_motor_index) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I don't see any issues with that. Can you do a PR?

@ndepal
Copy link
Contributor

ndepal commented Oct 17, 2019

That is possible. Do you have a setup where you can test it?

A quick bench test with camera triggering configured on pins 56 on a PixRacer (GPIO, active low) appears to work just fine.

@bkueng
Copy link
Member Author

bkueng commented Oct 17, 2019

Thanks for confirming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet