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

Timer N channel fix #9276

Merged
merged 3 commits into from
Oct 12, 2023
Merged

Timer N channel fix #9276

merged 3 commits into from
Oct 12, 2023

Conversation

shirase
Copy link
Contributor

@shirase shirase commented Sep 4, 2023

Maybe it fix problem with N timer channel for MATEKF405TE. I don`t have that board. Can someone test.

#8089

@MATEKSYS
Copy link
Contributor

MATEKSYS commented Sep 5, 2023

Thanks for the fix, We just did a test on bench. motor3 works when using "Master" slider in output tab or using transmitter after arming.
but when using the individual slider of motor "3". motor3 don't work.
and motor4 on Tim1 will beep when throttle back to 0. motor 1 and 2 don't.

I uploaded a video for your refference.
http://www.mateksys.com/Downloads/other/F405TE_S3.mp4

@shirase
Copy link
Contributor Author

shirase commented Sep 5, 2023

You compile it with #define USE_DSHOT_DMAR ?

@MATEKSYS
Copy link
Contributor

MATEKSYS commented Sep 5, 2023

You compile it with #define USE_DSHOT_DMAR ?

without Dshot burst. I tested it with normal DMA

@DzikuVx
Copy link
Member

DzikuVx commented Sep 5, 2023

I think we should switch this board to burst DMA in INAV 7

@MATEKSYS
Copy link
Contributor

MATEKSYS commented Sep 5, 2023

Just tested it again with #define USE_DSHOT_DMAR
Motor 3 and 4 work normal when using individual slider in outputs tab.

I think we should switch this board to burst DMA in INAV 7
I think so too

@shirase
Copy link
Contributor Author

shirase commented Sep 5, 2023

Added #define USE_DSHOT_DMAR for this board

@DzikuVx
Copy link
Member

DzikuVx commented Sep 5, 2023

@shirase how about extracting Burst DMA into a separate PR? I mean those are independent changes and looks like it does not work like expected. I will try to test it with a different board that has N timer channel only

@shirase
Copy link
Contributor Author

shirase commented Sep 5, 2023

@DzikuVx agree. It must to work without dma burst.

@DzikuVx
Copy link
Member

DzikuVx commented Sep 5, 2023

@shirase I mean we can switch this target to DMA Burst. But not as part of the fix for N channel timers

@shirase
Copy link
Contributor Author

shirase commented Sep 5, 2023

@DzikuVx Can you make PR for DMA burst targets? I do not have boards for testing. And i don`t known lists of problem boards.

@DzikuVx
Copy link
Member

DzikuVx commented Sep 5, 2023

I will do that

@rmaia3d
Copy link
Contributor

rmaia3d commented Sep 17, 2023

As I posted in #9277, even with those changes, the output from S1 to S4 on the Matek F405 Mini-TE board looks like this on an oscilloscope:
IMG_2731
(That's DSHOT 300 on zero throttle)

What draws the attention is S3 (scope Ch3, magenta line) being "inverted", a mirror of the other outputs. This is not unexpected, once ST's "N" timers are complementary and have inverted output levels from the "normal" channel.

But this is probably the source of the DSHOT errors. I don't fully recall the DSHOT spec, but I don't think it is tolerant to inverted logic levels.

@rmaia3d
Copy link
Contributor

rmaia3d commented Sep 19, 2023

Ok, so I realized that both #9265 and #9277 didn't include the fix in this PR. I manually added this PR to my testing code, flashed to the Matek F405 mini-TE board and checked S1 through S4 on the scope (DSHOT 300). This is what I got, at 0% and 50% throttle:

IMG_2901 copy
IMG_2902 copy

All 4 lines look corrected, without the mirroring (inverted signal) on S3. Yay!!

Next up will be installing this board in a quad and test flying. But, initially, I believe that this trio of PR's (#9265, #9276 and #9277) fixed the DSHOT issues on the miniTE board.

rmaia3d added a commit to rmaia3d/inav that referenced this pull request Sep 19, 2023
@shota3527
Copy link
Contributor

Has anyone tried Dshot beeper after these changes? It seems not working.

@rmaia3d
Copy link
Contributor

rmaia3d commented Sep 27, 2023

Has anyone tried Dshot beeper after these changes? It seems not working.

Yes, I can confirm Dshot beeper is not working, even in targets where DMA burst mode is not used/enabled. However, reverting #9265 makes it work again. Some change within that PR broke it for all targets.

Also, my test flight on the Matek F405miniTE with #9265, #9276 and #9277 didn't go well. The quad simply dropped less than 30 seconds after take-off, apparently with the motors being cut. Due to an error on my part, I didn't have blackbox logging enabled, but I do have DVR recording that shows the current dropping to less than 1A before impact and no flight mode changes (so not a failsafe, which was set to RTH or land if closer than 20m from home). Here's that DVR:

https://youtu.be/_No0jyui43s

I am still not sure if it could be directly related to the DMA code changes or a hardware issue or another issue altogether, but it does cast suspicion and doubt.

@rmaia3d rmaia3d mentioned this pull request Sep 27, 2023
@shirase
Copy link
Contributor Author

shirase commented Sep 28, 2023

@shota3527 @rmaia3d try this fix #9321. Can you check it on dshot600 and dshot300?

@shota3527 shota3527 mentioned this pull request Sep 27, 2023
@shota3527
Copy link
Contributor

shota3527 commented Oct 1, 2023

@shirase @rmaia3d
I have tried #9276, but did not manage to reproduce the motor stopping issue on the bench test
I have s1-s3 using dshot300/dshot600 on matekf405te_sd, S3 is TIM1 CH3N, Seems like it is working well
Beeper is also working on both DSHOT300 and 600 on current master

@DzikuVx
Copy link
Member

DzikuVx commented Oct 8, 2023

@shirase Is this still relevant? And working?

@mmosca
Copy link
Collaborator

mmosca commented Oct 8, 2023

I beleive @shota3527 got some reports it was working for one of the siwi engineers. I believe that was with this patch.

@shirase
Copy link
Contributor Author

shirase commented Oct 9, 2023

@DzikuVx I don't have board with N channel. Need feedback. It works or not.

@rmaia3d
Copy link
Contributor

rmaia3d commented Oct 10, 2023

@shirase Is this still relevant? And working?

As can be seen from the scope traces I posted a few comments above, this did indeed work to bring channel 3 to the "non-inverted" state. An inverted (0 being logic HIGH) DSHOT signal is how the ESC knows if the FC wants bi-directional comms or not. So, without this fix, 1 ESC (ch3) will work in bi-dir mode and the other 3 ESCs will work in normal mode. Not ideal.

All ESCs should work in the same mode, and by the scope traces above, this PR ensures that.

@rmaia3d
Copy link
Contributor

rmaia3d commented Oct 10, 2023

@shirase @rmaia3d
I have tried #9276, but did not manage to reproduce the motor stopping issue on the bench test
I have s1-s3 using dshot300/dshot600 on matekf405te_sd, S3 is TIM1 CH3N, Seems like it is working well
Beeper is also working on both DSHOT300 and 600 on current master

I'm away from home and wasn't able to test the changes after the dshot beeper fix, but before leaving I did further investigation and tests and was able to trace the motor stopping issue to the ESC itself.

It "overheats" and then stops the motors (actually it looks like it's a bug in the temperature cutoff protection or a faulty temp sensor). So it has nothing to do with the FC or Inav (I was able to reproduce the issue both on the bench and in-flight while running betaflight too). Good news!

@rmaia3d
Copy link
Contributor

rmaia3d commented Oct 10, 2023

Here's someone with a similar issue and another mention of the possible temperature cutoff bug. The ESC I'm using is the same, just the "mini" version.

https://discuss.ardupilot.org/t/no-throttle-with-props-on-hexa-680-13/80847/5

@mmosca
Copy link
Collaborator

mmosca commented Oct 10, 2023 via email

@DzikuVx DzikuVx added this to the 7.0 milestone Oct 12, 2023
@DzikuVx DzikuVx merged commit ad8e1a3 into iNavFlight:master Oct 12, 2023
14 checks passed
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.

6 participants