增加ppm RC驱动#200
Conversation
PeterJhon
force-pushed
the
drv_rc
branch
2 times, most recently
from
8ec3aa0 to
6dbaf61
Compare
There was a problem hiding this comment.
Pull request overview
Adds a PPM-based RC input driver for the Infineon edge-e83 (M33) target, wiring up a TCPWM capture channel and integrating the RC HAL/driver into the target build and early board initialization.
Changes:
- Enable
BSP_USING_RCand include RC HAL sources in the M33 build. - Add
drv_rcimplementation using TCPWM capture ISR to feed the PPM decoder and register anrcdevice. - Update ModusToolbox-generated clock/routing/peripheral config to provide the RC capture timer, pin routing, and clock divider.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| target/infineon/edge-e83/m33/rtconfig.h | Enables RC feature flag for this target. |
| target/infineon/edge-e83/m33/config/hal.py | Adds RC HAL sources to the build glob list. |
| target/infineon/edge-e83/m33/board/board.c | Calls drv_rc_init() during early init. |
| target/infineon/edge-e83/libs/TARGET_APP_KIT_PSE84_EVAL_EPC2/config/design.modus | Adds a TCPWM counter instance and routing for RC capture + related clock divider. |
| target/infineon/edge-e83/libs/TARGET_APP_KIT_PSE84_EVAL_EPC2/config/GeneratedSource/cycfg_routing.h | Generated trigger routing definitions for RC capture (and UART/DMA trigger changes). |
| target/infineon/edge-e83/libs/TARGET_APP_KIT_PSE84_EVAL_EPC2/config/GeneratedSource/cycfg_routing.c | Generated trigger mux connections including RC capture and UART/DMA triggers. |
| target/infineon/edge-e83/libs/TARGET_APP_KIT_PSE84_EVAL_EPC2/config/GeneratedSource/cycfg_peripherals.h | Adds RC_TIMER peripheral definitions and configurator exports. |
| target/infineon/edge-e83/libs/TARGET_APP_KIT_PSE84_EVAL_EPC2/config/GeneratedSource/cycfg_peripherals.c | Adds RC_TIMER counter config and divider assignment. |
| target/infineon/edge-e83/libs/TARGET_APP_KIT_PSE84_EVAL_EPC2/config/GeneratedSource/cycfg_peripheral_clocks.h | Adds the new peri_0_group_1_div_8_4 divider definitions. |
| target/infineon/edge-e83/libs/TARGET_APP_KIT_PSE84_EVAL_EPC2/config/GeneratedSource/cycfg_peripheral_clocks.c | Initializes the new divider to achieve the target timer frequency. |
| target/infineon/edge-e83/libraries/mtb-device-support-pse8xxgp/pdl/SConscript | Conditionally adds cy_tcpwm_counter.c when RC is enabled. |
| target/infineon/edge-e83/drivers/drv_rc.h | New RC driver header exposing drv_rc_init(). |
| target/infineon/edge-e83/drivers/drv_rc.c | New RC driver using TCPWM capture ISR + PPM decoder, registers rc device. |
| target/infineon/edge-e83/drivers/SConscript | Adds drv_rc.c to driver sources when RC is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static rt_err_t rc_init(rc_dev_t dev) | ||
| { | ||
| (void)dev; | ||
|
|
||
| RT_TRY(ppm_decoder_init(&ppm_decoder, TIMER_FREQ_HZ)); |
There was a problem hiding this comment.
This driver advertises SBUS support (includes sbus.h, defines an sbus_decoder, and hal_rc AUTO mode will probe SBUS first), but only initializes the PPM decoder and there’s no SBUS low-level init / UART ISR feeding sbus_input() + sbus_update(). As a result, selecting protocol = sbus via configuration will never produce updates. Either implement SBUS init+RX handling for this target, or remove the SBUS path and set the default protocol to PPM (and return an error for unsupported protocols).
| #include "cycfg_peripherals.h" | ||
|
|
||
| #ifndef min // mod by prife | ||
| #define min(x, y) (x < y ? x : y) |
There was a problem hiding this comment.
The local min macro is unsafe as written: it lacks parentheses around parameters/whole expression and can evaluate arguments multiple times. Prefer a fully-parenthesized macro (or an inline function) to avoid precedence/side-effect issues.
| #define min(x, y) (x < y ? x : y) | |
| #define min(x, y) (((x) < (y)) ? (x) : (y)) |
| Cy_TrigMux_Connect(PERI_0_TRIG_IN_MUX_0_SCB_TX_TR_OUT2, PERI_0_TRIG_OUT_MUX_0_PDMA0_TR_IN1, false, TRIGGER_TYPE_LEVEL); | ||
| Cy_TrigMux_Connect(PERI_0_TRIG_IN_MUX_0_SCB_TX_TR_OUT5, PERI_0_TRIG_OUT_MUX_0_PDMA0_TR_IN2, false, TRIGGER_TYPE_LEVEL); | ||
| Cy_TrigMux_Connect(PERI_0_TRIG_IN_MUX_3_PERI1_HSIOM_TR_OUT1, PERI_0_TRIG_OUT_MUX_3_TCPWM0_ALL_CNT_TR_IN27, false, TRIGGER_TYPE_LEVEL); |
There was a problem hiding this comment.
These trigger-mux connections change UART/DMA routing (e.g. SCB_TX_TR_OUT10 -> SCB_TX_TR_OUT2, plus new SCB5->PDMA connection) in addition to adding the RC capture route. Since the PR title/description is about adding a PPM RC driver, please confirm these UART/DMA routing changes are intentional; otherwise regenerate the config to include only the RC-related routing or split them into a separate PR.
| /* init RC */ | ||
| RT_CHECK(drv_rc_init()); | ||
|
|
There was a problem hiding this comment.
drv_rc_init() is called here, but drv_rc.h isn’t included in this file (unlike other board implementations). In C99+ this can fail to compile due to an implicit function declaration. Include drv_rc.h (or otherwise provide a prototype) before calling drv_rc_init().
|
|
||
| rt_err_t drv_rc_init(void) | ||
| { | ||
| RT_TRY(hal_rc_register(&rc_dev, "rc", RT_DEVICE_FLAG_RDWR, NULL)); |
|
|
||
| if (int_source & CY_TCPWM_INT_ON_CC0) { | ||
| uint32_t capture = Cy_TCPWM_Counter_GetCapture0Val(RC_TIMER_HW, RC_TIMER_NUM) + 1; | ||
| // rt_kprintf("%d\n", capture); |
|
|
||
| ppm_lock(&ppm_decoder); | ||
|
|
||
| for (uint8_t i = 0; i < min(rc->config.channel_num, ppm_decoder.total_chan); i++) { |
No description provided.