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

hwdef: SpeedyBeeF405-bdshot variant #28084

Closed
wants to merge 7 commits into from
Closed

Conversation

setupfpv
Copy link

I asked SpeedyBee to add bidshot support, sent them the edits, they promised to do it. A month has already passed, nothing has been added. So I decided to add it myself. Made by analogy with matek405wing-bidshot

@peterbarker
Copy link
Contributor

I asked SpeedyBee to add bidshot support, sent them the edits, they promised to do it. A month has already passed, nothing has been added. So I decided to add it myself. Made by analogy with matek405wing-bidshot

I don't think I've seen pull requests from SpeedyBee directly - they're definitely not ArduPilot partners. Feel frree to nudge them in either of those directions, or both :-)

@peterbarker
Copy link
Contributor

... also note this PR from @andyp1per which should be related; rearranging DMA channels is often for motor-output stuff, which you're playing with: https://github.com/ArduPilot/ardupilot/pull/22300/files

... please state what testing you've done on this PR. "I've flown it a lot" is good, for example...

I asked SpeedyBee to add bidshot support, sent them the edits, they promised to do it. A month has already passed, nothing has been added. So I decided to add it myself. Made by analogy with matek405wing-bidshot

Delete hwdef-bl.dat
@setupfpv
Copy link
Author

testing many times, also testing did some of my subscribers

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

LGTM

@peterbarker
Copy link
Contributor

testing many times, also testing did some of my subscribers

Thanks.

You might read up on git "topic branches", rather than committing to your master branch.

I think this can go in, just need @Hwurzburg to be OK with it, I think. Marking it for DevCall, anyway.

I asked SpeedyBee to add bidshot support, sent them the edits, they promised to do it. A month has already passed, nothing has been added. So I decided to add it myself. Made by analogy with matek405wing-bidshot

Delete hwdef-bl.dat
Copy link
Collaborator

@Hwurzburg Hwurzburg left a comment

Choose a reason for hiding this comment

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

I am not sure this has gotten full testing...tim4 is shared with spi3
also the comment is incorrect, UART3 does not have full DMA, but only UART1 requires it, and it still does, so the comment is incorrect and needs tp be changed
this needs Andys approval....and a squash

@setupfpv
Copy link
Author

I am not sure this has gotten full testing...tim4 is shared with spi3 also the comment is incorrect, UART3 does not have full DMA, but only UART1 requires it, and it still does, so the comment is incorrect and needs tp be changed this needs Andys approval....and a squash

Make it by compare matek405Wing-bdshot and start using on 4.4.5 So if in in SB405 is mistake, so then matek405Wing-bdshot also had mistake. I hade both FC, and now with dshot. SpeedybeeWing using on 4 planes with modifed fw. of couse was problem with bluejay at 4.5.x, before i used 4.4.3 as IMHO it's more stable. when bluejay support fixed, I upgraded to 4.5.5 but was problem with temp at 4.5.5, and i fixed it localy (in 4.5.6 i saw already fixed it, so now only editing in hdef was needed.).

@andyp1per
Copy link
Collaborator

I can't see how this will work well:

#define STM32_SPI_SPI2_RX_DMA_STREAM   STM32_DMA_STREAM_ID(1, 3)
#define STM32_SPI_SPI2_RX_DMA_CHAN     0
#define STM32_SPI_SPI2_TX_DMA_STREAM   STM32_DMA_STREAM_ID(1, 4) // shared SPI2_TX,USART3_TX,UART4_TX
#define STM32_SPI_SPI2_TX_DMA_CHAN     0
#define STM32_SPI_SPI3_RX_DMA_STREAM   STM32_DMA_STREAM_ID(1, 0) // shared TIM4_CH1,SPI3_RX
#define STM32_SPI_SPI3_RX_DMA_CHAN     0
#define STM32_SPI_SPI3_TX_DMA_STREAM   STM32_DMA_STREAM_ID(1, 5) // shared SPI3_TX,I2C1_RX
#define STM32_SPI_SPI3_TX_DMA_CHAN     0
#define STM32_TIM_TIM1_UP_DMA_STREAM   STM32_DMA_STREAM_ID(2, 5)
#define STM32_TIM_TIM1_UP_DMA_CHAN     6
#define STM32_TIM_TIM2_UP_DMA_STREAM   STM32_DMA_STREAM_ID(1, 1)
#define STM32_TIM_TIM2_UP_DMA_CHAN     3
#define STM32_TIM_TIM3_CH3_DMA_STREAM  STM32_DMA_STREAM_ID(1, 7) // shared TIM3_CH3,I2C1_TX,UART5_TX
#define STM32_TIM_TIM3_CH3_DMA_CHAN    5
#define STM32_TIM_TIM3_UP_DMA_STREAM   STM32_DMA_STREAM_ID(1, 2)
#define STM32_TIM_TIM3_UP_DMA_CHAN     5
#define STM32_TIM_TIM4_CH1_DMA_STREAM  STM32_DMA_STREAM_ID(1, 0) // shared TIM4_CH1,SPI3_RX
#define STM32_TIM_TIM4_CH1_DMA_CHAN    2
#define STM32_TIM_TIM4_UP_DMA_STREAM   STM32_DMA_STREAM_ID(1, 6)
#define STM32_TIM_TIM4_UP_DMA_CHAN     2

SPI locks both TX ad RX so you have SPI3 sharing with I2C (always bad) and TIM4_CH1

@andyp1per
Copy link
Collaborator

But yeah you are right, Matek has similar or worse problems, so if its working well for you it seems like a reasonable change.

@andyp1per
Copy link
Collaborator

Actually doing it this way is better:

PB7  TIM4_CH2  TIM4 PWM(1) GPIO(50) BIDIR
PB6  TIM4_CH1  TIM4 PWM(2) GPIO(51)
PB0  TIM3_CH3  TIM3 PWM(3) GPIO(52) 
PB1  TIM3_CH4  TIM3 PWM(4) GPIO(53) BIDIR

Can you try this? No conflict with I2C, the SPI device is OSD and because of fixes in 4.5 we get IC channel sharing with the UP channel on TIM3

@setupfpv
Copy link
Author

setupfpv commented Sep 15, 2024

Can you try this? No conflict with I2C, the SPI device is OSD and because of fixes in 4.5 we get IC channel sharing with the UP channel on TIM3

Did it. with blheli_s ESC with bluejay - all working. Compass working, motors arming and spinning, esc show rpm and temperature with:
SERVO_DSHOT_ESC,4
SERVO_AUTO_TRIM,1
SERVO_BLH_BDMASK,1

But on other model with esc blheli_32 didnt work motors, and esc didnt init with SERVO_DSHOT_ESC,1 and SERVO_DSHOT_ESC,3

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Sep 16, 2024
@peterbarker
Copy link
Contributor

Ping @andyp1per - your suggestion apparently stopped one of @setupfpv 's tests from working.

Thoughts?

@peterbarker
Copy link
Contributor

[10:25 AM]joshanne (James): On the same hand, do we have advice on bringing up a new board addressing the same issues?

@setupfpv
Copy link
Author

On the same hand, do we have advice on bringing up a new board addressing the same issues?

As I understand it, there are a lot of boards on F405 with the ending -dshot, then they are crooked, but available for downloading and use in the stable branch

@robustini
Copy link
Contributor

You will still have to shorten the board name I think.

"CHIBIOS_SHORT_BOARD_NAME must be 23 characters or less"

@setupfpv
Copy link
Author

"CHIBIOS_SHORT_BOARD_NAME must be 23 characters or less"

I wrote this, but the developers didn't like this option. It also doesn't work with the bootloader
+USE_BOOTLOADER_FROM_BOARD SpeedyBeeF405WING , if you plan to compile the stable release 4.5.6.
But in 4.6dev it works and compiles
That's why I just made a separate version for myself, which works fine, everything flies, there are no conflicts with sensors and I use it. And every time a stable version comes out, I download git, insert my edit, and compile.

@robustini
Copy link
Contributor

robustini commented Sep 17, 2024

This afternoon I do bench test on a plane fpv nice loaded as serials used.
But if it works on the MatekF405 Wing, I don't see why it shouldn't work here.

@andyp1per
Copy link
Collaborator

But on other model with esc blheli_32 didnt work motors, and esc didnt init with SERVO_DSHOT_ESC,1 and SERVO_DSHOT_ESC,3

I don't see how this can be related, the BlueJay and BLHeli32 code paths are the same, its just the pulse widths that are different. Are you saying BLHeli32 worked with your original changes?

@setupfpv
Copy link
Author

But on other model with esc blheli_32 didnt work motors, and esc didnt init with SERVO_DSHOT_ESC,1 and SERVO_DSHOT_ESC,3

I don't see how this can be related, the BlueJay and BLHeli32 code paths are the same, its just the pulse widths that are different. Are you saying BLHeli32 worked with your original changes?

Yep, partialy. If i connect usb cable and waiting FC init, and then connect battery, then ESC with Blheli_32 initialise. But if i only connect battery, will not. It looks like BUG with bluejay ESC before ardupilot 4.5.5 fw.

@andyp1per
Copy link
Collaborator

But on other model with esc blheli_32 didnt work motors, and esc didnt init with SERVO_DSHOT_ESC,1 and SERVO_DSHOT_ESC,3

I don't see how this can be related, the BlueJay and BLHeli32 code paths are the same, its just the pulse widths that are different. Are you saying BLHeli32 worked with your original changes?

Yep, partialy. If i connect usb cable and waiting FC init, and then connect battery, then ESC with Blheli_32 initialise. But if i only connect battery, will not. It looks like BUG with bluejay ESC before ardupilot 4.5.5 fw.

So I am confused, are my proposed changes good or not?

@setupfpv
Copy link
Author

So I am confused, are my proposed changes good or not?

Well, it depends on how you look at it. On the one hand, the point of a dshot is to get RPM and temperature data from the regulator on the Blheli_S/Bluejay. For the Blheli_32 there is already a telemetry port.
Therefore, if you use it as intended, then everything is fine.
On the other hand, the firmware is not universal and does not work properly for those who have a Blheli_32, there is no way to check it on the AM32.
.. but then we still need to find a bug, why during initialization the regulator is not detected, but is detected after the ardupilot is loaded and then power is supplied to the regulator.

@andyp1per
Copy link
Collaborator

So I am confused, are my proposed changes good or not?

Well, it depends on how you look at it. On the one hand, the point of a dshot is to get RPM and temperature data from the regulator on the Blheli_S/Bluejay. For the Blheli_32 there is already a telemetry port. Therefore, if you use it as intended, then everything is fine. On the other hand, the firmware is not universal and does not work properly for those who have a Blheli_32, there is no way to check it on the AM32. .. but then we still need to find a bug, why during initialization the regulator is not detected, but is detected after the ardupilot is loaded and then power is supplied to the regulator.

I'm still confused, what is working and what is not

@setupfpv
Copy link
Author

I'm still confused, what is working and what is not

All working. But with blheli32, only if you powerUP ESC after powerUP and boot FC.

@andyp1per
Copy link
Collaborator

I'm still confused, what is working and what is not

All working. But with blheli32, only if you powerUP ESC after powerUP and boot FC.

This won't be related to the DMA allocation. BLHeli has a complicated signal detection scheme that relies on receiving a number of good frames over a certain period. Lot's of different things can interfere with this.

Copy link
Collaborator

@andyp1per andyp1per left a comment

Choose a reason for hiding this comment

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

Still think this should be folded into the existing firmware, but if @Hwurzburg is ok with it like this then I am ok

@Hwurzburg
Copy link
Collaborator

since this now does not impact dshot on other outputs should just be a change to std firmware and not separate target

@robustini
Copy link
Contributor

robustini commented Sep 17, 2024

Tested on the bench, found no problems.
If we put it in the firmware standard then on the other boards we should do it imho as well, so in my opinion it would be better to put it separately.
Isn't it simpler to increase the 23-character limit or would it have catastrophic repercussions?

@setupfpv
Copy link
Author

found no problems.

and what about blheli_32?

Copy link
Collaborator

@Hwurzburg Hwurzburg left a comment

Choose a reason for hiding this comment

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

Andy and I agree this should just be a change to the basic SpeedyBeeeF405WING....I have done it for you here #28137, to avoid having you do it and given you commit credit (in case you want it)...just need Andy to final approve

@robustini
Copy link
Contributor

robustini commented Sep 18, 2024

found no problems.

and what about blheli_32?

found no problems.

and what about blheli_32?

I tested your original implementation and had no problems with BLHeli32, if you want I will also try with AM32.
I tried that one since you reported that it had already been tested by you and your subscribers.
As telemetry data it only detects RPM, not temperature as well, now I don't remember if the bdshot was capable of providing anything else as well, this ESC in telemetry obviously also provides voltage and temperature when connected to a UART.
Motor connected here:

PB0 TIM3_CH3 TIM3 PWM(3) GPIO(52) BIDIR

TIM3 it is true that it is also shared with I2C1_TX channel but I did not detect any problems, although potentially there could be.
I didn't understand if you had the problem with BLHeli32 only with the right modifications suggested by @andyp1per.

@tridge
Copy link
Contributor

tridge commented Sep 18, 2024

replaced by #28137

@tridge tridge closed this Sep 18, 2024
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.

6 participants