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 Aocoda-RC-H743Dual target board configuration #25396

Closed
wants to merge 8 commits into from

Conversation

aocodarc
Copy link

Add Aocoda-RC-H743Dual target board configuration

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.

  • need to reserve your board id in a separate pr immediately unless this board really is the TWD_H7

  • if this is to be commercially available and included in the wiki, you need a complete README.md file with pinouts, images,etc.

@aocodarc
Copy link
Author

  • need to reserve your board id in a separate pr immediately unless this board really is the TWD_H7

OK, we prepared board id reserve #25398, please check.

  • if this is to be commercially available and included in the wiki, you need a complete README.md file with pinouts, images,etc.

OK, when board id for AocodaRC H743 Dual is accepted, i'll prepare readme and other stuff asap.

BTW, Is there any guide or links can help us to add a page to ardupilot wiki?

@Hwurzburg
Copy link
Collaborator

here is the general format https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_HAL_ChibiOS/hwdef/KakuteH7-Wing/README.md
here is how to setup to do big edits (like a new page for an autopilot) https://ardupilot.org/dev/docs/common-wiki_editing_guide.html

@peterbarker
Copy link
Contributor

@aocodarc please see https://ardupilot.org/dev/docs/git-interactive-rebase.html for tips on cleaning up the commit list. Don't pull master into your branch, rebase on top of master.

@aocodarc
Copy link
Author

aocodarc commented Oct 30, 2023

@aocodarc please see https://ardupilot.org/dev/docs/git-interactive-rebase.html for tips on cleaning up the commit list. 》Don't pull master into your branch, rebase on top of master.

Sorry! Still not very proficient.

Should I rebase these commits?

@lida2003 lida2003 force-pushed the Aocoda-RC-H743Dual branch 2 times, most recently from 54d5ddc to 39a675b Compare October 30, 2023 08:52
@aocodarc
Copy link
Author

@Hwurzburg @peterbarker @drnic OK git rebased. Please check, thanks.

@lida2003
Copy link
Contributor

lida2003 commented Oct 30, 2023

It needs two commits(#25401, #25402) related to Aocoda-RC board id, or CI build fails.

commit 7ddcd7ab0c885d8d5aabdb81c1d54d488417df34
Author: Daniel Li <[email protected]>
Date:   Mon Oct 30 11:50:25 2023 +0800

    AP_Bootloader: Reserve Aocoda-RC board IDs and apply for H743DUAL/F405V3

commit 6f2236e3515d79b5a327132efdb7b9322a032e32
Author: daniel.li <[email protected]>
Date:   Mon Oct 30 10:26:31 2023 +0800

    AP_Bootloader: Fix AIRVOLUTE format issue


@lida2003 lida2003 force-pushed the Aocoda-RC-H743Dual branch 2 times, most recently from 2fa049e to 1b816f8 Compare November 1, 2023 06:54
@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Nov 1, 2023
PA12 OTG_FS_DP OTG1

# OTG USB Detect
PE2 EXT_CS2 CS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this usually VBUS?

Copy link
Contributor

@lida2003 lida2003 Nov 2, 2023

Choose a reason for hiding this comment

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

It's VBUS actually. I'm NOT sure if the configuration should be this way, just reference Matek H743

# external CS pins
PD4 EXT_CS1 CS
PE2 EXT_CS2 CS

Copy link
Contributor

Choose a reason for hiding this comment

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

Or should I configure this as below?

PE2 VBUS INPUT

Copy link
Collaborator

Choose a reason for hiding this comment

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

right, that's what I was thinking

Copy link
Contributor

Choose a reason for hiding this comment

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

Will do

Copy link
Collaborator

Choose a reason for hiding this comment

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

not changed yet,fyi

Copy link
Contributor

@lida2003 lida2003 Nov 6, 2023

Choose a reason for hiding this comment

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

I have checked again (it should be update 9 hours ago, but don't know why). @Hwurzburg Can you help to check it again.

PA3 TIM5_CH4 TIM5 PWM(6) GPIO(55) //S6
PD12 TIM4_CH1 TIM4 PWM(7) GPIO(56) BIDIR //S7
PD13 TIM4_CH2 TIM4 PWM(8) GPIO(57) //S8
PD14 TIM4_CH3 TIM4 PWM(9) GPIO(58) NODMA //S9
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can't do this if you want dshot on PWM(7) and PWM(8). You should take off the NODMA

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it because these pins are all PDx? And if I remove "NODMA", PWM(9) PWM(10) can also be use as bdshot, right?

change from

PD12 TIM4_CH1 TIM4  PWM(7)  GPIO(56) BIDIR   //S7
PD13 TIM4_CH2 TIM4  PWM(8)  GPIO(57)         //S8
PD14 TIM4_CH3  TIM4  PWM(9)  GPIO(58) NODMA  //S9
PD15 TIM4_CH4  TIM4  PWM(10) GPIO(59) NODMA  //S10

to

PD12 TIM4_CH1 TIM4  PWM(7)  GPIO(56) BIDIR   //S7
PD13 TIM4_CH2 TIM4  PWM(8)  GPIO(57)         //S8
PD14 TIM4_CH3  TIM4  PWM(9)  GPIO(58)        //S9
PD15 TIM4_CH4  TIM4  PWM(10) GPIO(59)        //S10

Copy link
Collaborator

Choose a reason for hiding this comment

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

To enable bi-dir on all you would need to add BIDIR to PWM(9) as well. More what I was saying was that dshot won't work at all on PWM7-10 without this change

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it because these pins are all PDx

No, its because they are all on TIM4

Copy link
Contributor

@lida2003 lida2003 Nov 2, 2023

Choose a reason for hiding this comment

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

To enable bi-dir on all you would need to add BIDIR to PWM(9) as well.

OK. I though PWM(1) to PWM(8) works for bdshot is OK, as there are only two motor connectors wiring PWM(1) to PWM(8).

I'm NOT aware of how code handling bdshot. Is BIDIR lable working on TIM4_CHn and TIM4_CHn+1, n belongs to odd numbers?

More what I was saying was that dshot won't work at all on PWM7-10 without this change

Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's missing. Thanks.

图片

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, but you should add BIDIR to TIM4_CH3 - we don't currently have the ability to do split dshot/bdshot on the same timer

Copy link
Contributor

@lida2003 lida2003 Nov 5, 2023

Choose a reason for hiding this comment

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

Currently the layout has following suggestions:
a) S1-S8 output throught ESC connector, which is supposed to be used as bdshot by default.
b) S9 S10 used for servos
c) SE1, SE2 used for servos or pinio (which has a current limit resistor)

So I have left these IO definitions below:

PB0  TIM3_CH3 TIM3  PWM(1)  GPIO(50) BIDIR   //S1
PB1  TIM3_CH4 TIM3  PWM(2)  GPIO(51)         //S2
PA0  TIM2_CH1 TIM2  PWM(3)  GPIO(52) BIDIR   //S3
PA1  TIM2_CH2 TIM2  PWM(4)  GPIO(53)         //S4
PA2  TIM5_CH3 TIM5  PWM(5)  GPIO(54) BIDIR   //S5
PA3  TIM5_CH4 TIM5  PWM(6)  GPIO(55)         //S6
PD12 TIM4_CH1 TIM4  PWM(7)  GPIO(56) BIDIR   //S7
PD13 TIM4_CH2 TIM4  PWM(8)  GPIO(57)         //S8
PD14 TIM4_CH3  TIM4  PWM(9)  GPIO(58)        //S9
PD15 TIM4_CH4  TIM4  PWM(10) GPIO(59)        //S10
PE5  TIM15_CH1 TIM15 PWM(11) GPIO(60) NODMA  //SE1
PE6  TIM15_CH2 TIM15 PWM(12) GPIO(61) NODMA  //SE2

Copy link
Collaborator

Choose a reason for hiding this comment

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

this stikk needs the change Andy mentioned to add BIDIR to PD14 line

Copy link
Contributor

Choose a reason for hiding this comment

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

@Hwurzburg @andyp1per OK, I'll add BIDIR to PD14 line. But I don't understand why?

As andy and I have discussion about this and in his words "BIDIR applies to channel pairs - either CH1-2 or CH3-4. It can be on any channel apart from TIM4_CH4 which does not exist."

Now I just wana use S1-S8 left S9/S10 no bdshot (maybe saving DMA), why I have to add BIDIR to PD14?

One more question if I add BIDIR to PD14, does PD14/PD15 (S9/S10) can use as bdshot pins?

ROMFS_WILDCARD libraries/AP_OSD/fonts/font*.bin

# support VTX IRC tramp protocol
define AP_TRAMP_ENABLED 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is enabled by default on 2MB

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I'll remove the tramp. Thanks for the tip.

@@ -0,0 +1,4 @@
# logging
LOG_BACKEND_TYPE 4
LOG_FILE_BUFSIZE 4
Copy link
Collaborator

Choose a reason for hiding this comment

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

that's way too small

Copy link
Contributor

@lida2003 lida2003 Nov 2, 2023

Choose a reason for hiding this comment

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

I found most of LOG_FILE_BUFSIZE is set to 16.

So how about 16? what if I doesn't define LOG_FILE_BUFSIZE, is there any default values?

Maybe I can remove defaults.parm file.

LOG_FILE_BUFSIZE    16

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think remove defaults.parm - the defaults should be set appropriately based on chip (200 for H743 I believe)

Copy link
Contributor

@lida2003 lida2003 Nov 2, 2023

Choose a reason for hiding this comment

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

OK, Thanks a lot. I'll remove the file.
It seems the param is quite advanced. Just default(based on MCU) will do.

#ifndef HAL_LOGGING_FILE_BUFSIZE
#if HAL_MEM_CLASS >= HAL_MEM_CLASS_1000
#define HAL_LOGGING_FILE_BUFSIZE 200
#elif HAL_MEM_CLASS >= HAL_MEM_CLASS_500
#define HAL_LOGGING_FILE_BUFSIZE 80
#elif HAL_MEM_CLASS >= HAL_MEM_CLASS_300
#define HAL_LOGGING_FILE_BUFSIZE 50
#else
#define HAL_LOGGING_FILE_BUFSIZE 16
#endif
#endif

/*
memory classes, in kbytes. Board must have at least the given amount
of memory
*/
#define HAL_MEM_CLASS_20 1
#define HAL_MEM_CLASS_64 2
#define HAL_MEM_CLASS_192 3
#define HAL_MEM_CLASS_300 4
#define HAL_MEM_CLASS_500 5
#define HAL_MEM_CLASS_1000 6

#if HAL_MEMORY_TOTAL_KB >= 1000
#define HAL_MEM_CLASS HAL_MEM_CLASS_1000
#elif HAL_MEMORY_TOTAL_KB >= 500
#define HAL_MEM_CLASS HAL_MEM_CLASS_500
#elif HAL_MEMORY_TOTAL_KB >= 300
#define HAL_MEM_CLASS HAL_MEM_CLASS_300
#elif HAL_MEMORY_TOTAL_KB >= 192
#define HAL_MEM_CLASS HAL_MEM_CLASS_192
#elif HAL_MEMORY_TOTAL_KB >= 64
#define HAL_MEM_CLASS HAL_MEM_CLASS_64
#else
#define HAL_MEM_CLASS HAL_MEM_CLASS_20
#endif

total_memory = 0
for (address, size, flags) in ram_map:
size *= 1024
cc_regions.append('{0x%08x, 0x%08x, CRASH_CATCHER_BYTE }' % (address, address + size))
if self.env_vars['APP_RAM_START'] is not None and address == ram_map[self.env_vars['APP_RAM_START']][0]:
ram_reserve_start = self.get_ram_reserve_start()
address += ram_reserve_start
size -= ram_reserve_start
regions.append('{(void*)0x%08x, 0x%08x, 0x%02x }' % (address, size, flags))
total_memory += size
f.write('#define HAL_MEMORY_REGIONS %s\n' % ', '.join(regions))
f.write('#define HAL_CC_MEMORY_REGIONS %s\n' % ', '.join(cc_regions))
f.write('#define HAL_MEMORY_TOTAL_KB %u\n' % (total_memory/1024))

Copy link
Collaborator

Choose a reason for hiding this comment

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

now you will need the defaults.parm file to setup the LED, now that it can work and you have it marked as such on new board

Copy link
Contributor

Choose a reason for hiding this comment

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

Welp, I don't know how to, any links or idea about setting LED in defaults.parm file?

Copy link
Collaborator

@Hwurzburg Hwurzburg Nov 6, 2023

Choose a reason for hiding this comment

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

add to a defaults.param file
# setup for LEDs on chan13
SERVO13_FUNCTION 120

Copy link
Contributor

@lida2003 lida2003 Nov 7, 2023

Choose a reason for hiding this comment

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

OK, SERVO13 for PWM(13), thanks very much!

@Hwurzburg
Copy link
Collaborator

you are failing ci since you have not added the bootloader in Tools/bootloaders...fyi

@lida2003
Copy link
Contributor

lida2003 commented Nov 2, 2023

@Hwurzburg

you are failing ci since you have not added the bootloader in Tools/bootloaders...fyi

Yes, it needs two commits(#25401, #25402).

When I create this PR, I'm NOT aware of board id. How can I make CI work in this PR?

commit 7ddcd7ab0c885d8d5aabdb81c1d54d488417df34
Author: Daniel Li <[email protected]>
Date:   Mon Oct 30 11:50:25 2023 +0800

    AP_Bootloader: Reserve Aocoda-RC board IDs and apply for H743DUAL/F405V3

commit 6f2236e3515d79b5a327132efdb7b9322a032e32
Author: daniel.li <[email protected]>
Date:   Mon Oct 30 10:26:31 2023 +0800

    AP_Bootloader: Fix AIRVOLUTE format issue


@Hwurzburg
Copy link
Collaborator

sorry all my review comments had been pending me pushing the final review button...so they have been there for days but were not shown...

@lida2003
Copy link
Contributor

lida2003 commented Nov 4, 2023

sorry all my review comments had been pending me pushing the final review button...so they have been there for days but were not shown...

I have rebased all the above changes. And listed question below:

  1. VBUS config

It's just reference Matek H743, should I change to PE2 VBUS INPUT?

# external CS pins
PD4 EXT_CS1 CS
PE2 EXT_CS2 CS

  1. is it possible that Rx1 can be configured as SBUS (for DJI) or CRSF (for ELRS), and at the mean time LED stripe can work properly?

@Hwurzburg You mean config as below can work both with SBUS and CRSF?

If so, how can I configure two different setting? Just belwo works?
SERIAL1_OPTIONS = 23 for SBUS
SERIAL1_OPTIONS = 0 for CRSF

# USART1 (RC input), SERIAL1
PA9  USART1_TX USART1
PA10 USART1_RX USART1
define DEFAULT_SERIAL1_PROTOCOL SerialProtocol_RCIN
  1. Just a quick quesition, is it possible to apply BIDIR to channel pairs PB8/PB9 or PD14/PD15, which use TIM4_CH3&TIME_CH4?

图片

图片

  1. There are two commits(AP_Bootloader: Fix AIRVOLUTE format issue #25401, AP_Bootloader: Reserve Aocoda-RC board IDs and apply for H743DUAL/F405V3 #25402) for this PR, as when I create this PR, I'm NOT aware of board id. How can I make CI work in this PR. So How can I make this PR pass CI build? Or I have to create a new PR?

@lida2003
Copy link
Contributor

lida2003 commented Nov 4, 2023

@Hwurzburg "Andy, he meant SBus, and TX1 is pinned out" I doesn't understand this. Can you explain a little bit more?

@Hwurzburg
Copy link
Collaborator

@Hwurzburg "Andy, he meant SBus, and TX1 is pinned out" I doesn't understand this. Can you explain a little bit more?

Andy had asked what you meant by "subs" in a comment, I was responding to him...look thru the history of comments ...remember I actually made the comment two days ago, but because I forgot to complete the review, they only appeared in the flow now...

@Hwurzburg
Copy link
Collaborator

Hwurzburg commented Nov 5, 2023

sorry all my review comments had been pending me pushing the final review button...so they have been there for days but were not shown...

I have rebased all the above changes. And listed question below:

  1. VBUS config

It's just reference Matek H743, should I change to PE2 VBUS INPUT?

# external CS pins
PD4 EXT_CS1 CS
PE2 EXT_CS2 CS

  1. is it possible that Rx1 can be configured as SBUS (for DJI) or CRSF (for ELRS), and at the mean time LED stripe can work properly?

@Hwurzburg You mean config as below can work both with SBUS and CRSF?

If so, how can I configure two different setting? Just belwo works? SERIAL1_OPTIONS = 23 for SBUS SERIAL1_OPTIONS = 0 for CRSF

# USART1 (RC input), SERIAL1
PA9  USART1_TX USART1
PA10 USART1_RX USART1
define DEFAULT_SERIAL1_PROTOCOL SerialProtocol_RCIN
  1. Just a quick quesition, is it possible to apply BIDIR to channel pairs PB8/PB9 or PD14/PD15, which use TIM4_CH3&TIME_CH4?

图片

图片

  1. There are two commits(AP_Bootloader: Fix AIRVOLUTE format issue #25401, AP_Bootloader: Reserve Aocoda-RC board IDs and apply for H743DUAL/F405V3 #25402) for this PR, as when I create this PR, I'm NOT aware of board id. How can I make CI work in this PR. So How can I make this PR pass CI build? Or I have to create a new PR?

answers to your questions:
1, Andy already addressed this...should really be VBUS INPUT
2.yes..you have it right in the PR now...works for everything except PPM..and LED will work...no SERIAL_OPTIONS changes (except for FPort and SRXL2 which use half duplex and user must configure this for those))
3. Andy will need to address this, hes the bdir dshot guy
4. before CI will pass you need add the bootloader

@lida2003
Copy link
Contributor

lida2003 commented Nov 5, 2023

  1. before CI will pass you need to reserve the board id in https://github.com/ArduPilot/ardupilot/blob/0b1c6eec16aef38ae9134b427ab1b4c6c1d6e489/Tools/AP_Bootloader/board_types.txt in a separate PR that would need to be merged first...I suggest adding this at line 256 in that file and pushing a PR:
    AP_HW_AOCODA-RC-H743DUAL 1142

Currently, I have reserved board id in these two commits(#25401, #25402)

How can I make CI work in this PR? It seems I can't use cherry-pick.

--- update

It seems work, eventually failed.

Running (python waf configure --board thepeach-r1 --bootloader)
Failed builds:
  configure: Aocoda-RC-H743Dual
Error: Process completed with exit code 1.

@Hwurzburg
Copy link
Collaborator

@andyp1per please approve if okay now..

@Hwurzburg
Copy link
Collaborator

since changes were not enabled, I squashed, rebased, and split it here #25648

@tridge
Copy link
Contributor

tridge commented Nov 27, 2023

@aocodarc merged the cleaned up commits from @Hwurzburg

@tridge tridge closed this Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hwdef WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants