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

Bi-directional dshot on iomcu F103 8MHz #27087

Merged

Conversation

burgeruser
Copy link
Contributor

This PR adds support for bdshot telemetry on iomcus using the F103 with 8Mhz. There is one limitation - only channels 1-4,7-8 support telemetry. It is not possible to support 5-6 as these share a DMA channel with the fast UART interconnect.

Support is built into the iomcu dshot variant so to use you simply need to set BRD_IO_DSHOT and supply the appropriate SERVO_BLH_BDMASK. ESC telemetry indexes are modified to reflect the fact that ESC's 1-4 and 7-8 can now provide telemetry.

EDT is working, so you can get temperature, current and voltage information from the ESCs using bdshot as well. To use set SERVO_DSHOT_ESC in the normal way.

The two binary files named "iofirmware_f103_8MHz_dshot_lowpolh.bin" and iofirmware_f103_8MHz_dshot_highpolh.bin" were compiled using Copter-4.5.2.

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.

Very nice!

I think its better to put a hwdef.inc in iomcu-f103-dshot and then reference that from both places in order to avoid code duplication

@burgeruser
Copy link
Contributor Author

Very nice!

I think its better to put a hwdef.inc in iomcu-f103-dshot and then reference that from both places in order to avoid code duplication

I've just tested on Podman H7 flight controller, and everything is currently working as expected.

@andyp1per
Copy link
Collaborator

Great!

Still need the inc change

@burgeruser burgeruser force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from bc64426 to fb953e0 Compare May 25, 2024 02:35
@burgeruser
Copy link
Contributor Author

Great!

Still need the inc change
this

I think its better to put a hwdef.inc in iomcu-f103-dshot and then reference that from both places in order to avoid code duplication.

Before:
before

After I have changed:
after

@@ -0,0 +1,93 @@
# hw definition file for processing by chibios_pins.py

include ../iomcu/hwdef.inc
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to almost exactly match the old hwdef.dat. You have added a whole bunch of extra lines - please remove them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to almost exactly match the old hwdef.dat. You have added a whole bunch of extra lines - please remove them
remove them


include ../iomcu/hwdef.inc

# defines to allow the loop timing to be observed with a Saleae
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should include iomcu-f103-dshot/hwdef.inc and almost all the lines should be deleted

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing actually includes this so I don't think there is a need for another hwdef.inc - just use hwdef.dat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right - just use hwdef.dat in iomcu-f103-8Mhz-dshot/hwdef.dat.

@burgeruser burgeruser force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from 5a142b4 to 4d9347f Compare May 25, 2024 12:21
@burgeruser burgeruser requested a review from andyp1per May 25, 2024 12:23
@burgeruser
Copy link
Contributor Author

I recently made modifications to the file "iomcu-f103-dshot/hwdef.inc".
When editing, I aimed to follow the previous coding style of Ardupilot and maintained the order of define consistently, ensuring clarity for readers.
As a result, redundant .inc suffix files should no longer be necessary.
I have deleted all the lines should be deleted in file "iomcu-f103-dshot/hwdef.inc",and modified "iomcu-f103-dshot/hwdef.dat", "iomcu-f103-8Mhz-dshot/hwdef.dat" files, just use hwdef.dat in iomcu-f103-8Mhz-dshot/hwdef.dat.
after

@andyp1per andyp1per force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from 3f85a57 to 2e5b9df Compare May 25, 2024 21:40
@andyp1per
Copy link
Collaborator

I've fixed it for you - please check it

@burgeruser
Copy link
Contributor Author

I am currently attempting to test with an octocopter.

@burgeruser
Copy link
Contributor Author

I understand that code style encourages minimizing redundant code by leveraging encapsulation. Key to this principle is the use of functions, methods, classes, or modules, which allow for reusability. Include or import statements are commonly employed to incorporate external code or modules, facilitating the reuse of existing functionalities, thereby reducing the likelihood of errors and enhancing development efficiency.
after

@burgeruser
Copy link
Contributor Author

The two binary files named "iofirmware_f103_8MHz_dshot_lowpolh.bin" and iofirmware_f103_8MHz_dshot_highpolh.bin" were compiled using Copter-4.5.2.
After a careful compilation on my computer, I successfully tested with the Podman H7 flight controller using the iomcu output for the 8-channel DShot signal on an octocopter. The results were flawless, allowing for proper motor speed feedback.
test

@andyp1per andyp1per force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from 04e0375 to 0a65cd6 Compare May 26, 2024 08:06
@andyp1per
Copy link
Collaborator

The build script needed updating and I also needed to make sure that the binaries were compiled with the right compiler - so I have fixed this now. Please check it.

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.

Commits now broken

@andyp1per
Copy link
Collaborator

You now need to squash and merge your commit into the other one

@burgeruser
Copy link
Contributor Author

I recently standardized the folder names to 8MHz, ensuring no compilation errors. Earlier issues arose due to inconsistent naming in the path folders, specifically "iomcu-f103-8Mhz-dshot". I addressed the Python file validation issue related to this, and now, I've resolved the root problem by renaming the folders to "iomcu-f103-8MHz-dshot".

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.

You should change everything to use MHz, both to be consistent with the other directory and because that is the correct capitalisation

@burgeruser burgeruser force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from 5398a0b to a94b388 Compare May 26, 2024 10:54
@burgeruser burgeruser changed the title Bi-directional dshot on iomcu F103 8Mhz Bi-directional dshot on iomcu F103 8MHz May 26, 2024
@burgeruser burgeruser force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from a94b388 to 085e628 Compare May 26, 2024 11:38
@burgeruser
Copy link
Contributor Author

burgeruser commented May 26, 2024

You now need to squash and merge your commit into the other one

This time, my modifications were based on the latest master branch. I then compressed and merged all my commits into a single new commit.

This branch is 1 commit ahead of master.

@burgeruser burgeruser requested a review from andyp1per May 26, 2024 13:02
@robsenseuser
Copy link
Contributor

Unluckily, my PR submission was merged ahead of today.

@andyp1per andyp1per force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from 7bbe7b2 to a1c4476 Compare May 27, 2024 19:54
@andyp1per
Copy link
Collaborator

andyp1per commented May 27, 2024

You need to learn how to use git. I have redone the commits for you so that they have the right messages and are split appropriately. Please don't change anything now. I have also rebuilt the firmware so that it is using the right branch and compiler.

@andyp1per andyp1per force-pushed the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch from a1c4476 to 5e72aea Compare May 27, 2024 20:11
@tridge tridge merged commit 60493fd into ArduPilot:master May 27, 2024
44 checks passed
@burgeruser
Copy link
Contributor Author

You need to learn how to use git. I have redone the commits for you so that they have the right messages and are split appropriately. Please don't change anything now. I have also rebuilt the firmware so that it is using the right branch and compiler.

Thank you very much, this is my first successful merge with Ardupilot's code, it's been a fantastic experience.

@burgeruser burgeruser deleted the burgeruser-bdshot-for-f103-8Mhz-iofirmware branch May 28, 2024 03:34
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.

None yet

5 participants