-
Notifications
You must be signed in to change notification settings - Fork 18k
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
Conversation
There was a problem hiding this 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.
OK, we prepared board id reserve #25398, please check.
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? |
here is the general format https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_HAL_ChibiOS/hwdef/KakuteH7-Wing/README.md |
@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? |
54d5ddc
to
39a675b
Compare
@Hwurzburg @peterbarker @drnic OK git rebased. Please check, thanks. |
It needs two commits(#25401, #25402) related to Aocoda-RC board id, or CI build fails.
|
2fa049e
to
1b816f8
Compare
PA12 OTG_FS_DP OTG1 | ||
|
||
# OTG USB Detect | ||
PE2 EXT_CS2 CS |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
ardupilot/libraries/AP_HAL_ChibiOS/hwdef/MatekH743/hwdef.dat
Lines 48 to 50 in 7d4d2f9
# external CS pins | |
PD4 EXT_CS1 CS | |
PE2 EXT_CS2 CS |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not changed yet,fyi
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
ardupilot/libraries/AP_Logger/AP_Logger.cpp
Lines 26 to 36 in 1bf7c9e
#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 |
ardupilot/libraries/AP_HAL/AP_HAL_Boards.h
Lines 107 to 116 in 1bf7c9e
/* | |
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 |
ardupilot/libraries/AP_HAL/board/chibios.h
Lines 7 to 19 in 1bf7c9e
#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 |
ardupilot/libraries/AP_HAL_ChibiOS/hwdef/scripts/chibios_hwdef.py
Lines 1101 to 1113 in 1bf7c9e
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)) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
libraries/AP_HAL_ChibiOS/hwdef/Aocoda-RC-H743Dual/defaults.parm
Outdated
Show resolved
Hide resolved
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?
|
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... |
f042273
to
68b3a90
Compare
I have rebased all the above changes. And listed question below:
It's just reference Matek H743, should I change to ardupilot/libraries/AP_HAL_ChibiOS/hwdef/MatekH743/hwdef.dat Lines 48 to 50 in 7d4d2f9
@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?
|
@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... |
answers to your questions: |
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.
|
@andyp1per please approve if okay now.. |
since changes were not enabled, I squashed, rebased, and split it here #25648 |
@aocodarc merged the cleaned up commits from @Hwurzburg |
Add Aocoda-RC-H743Dual target board configuration