Update ChibiOS HAL to v9.0 #3202
Conversation
josesimoes
added
the
Platform: STM32
WalkthroughBumped the HAL configuration version macro from CHIBIOS_HAL_CONF_VER_8_4 to CHIBIOS_HAL_CONF_VER_9_0 in multiple target halconf.h files; one file also had a minor end-of-file formatting adjustment. Changes
Sequence Diagram(s)(omitted — changes are simple macro updates, no control-flow modifications) Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
nfbot
added
the
Type: dependencies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
targets/ChibiOS/ORGPAL_PALTHREE/nanoCLR/halconf.h (1)
312-318: New I2C slave-mode toggle added — OK. Consider tightening the docstring.Doc says “Enables the I2C slave subsystem,” but this macro indicates platform support rather than enabling a subsystem by itself.
Optionally adjust wording:
/** - * @brief Enables the I2C slave subsystem. + * @brief Target supports I2C slave mode. + * @note This is a capability flag; enabling and usage are controlled elsewhere. */If this will be defined in many halconf.h files, consider moving a default to halconf_nf.h to reduce duplication.
targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoCLR/halconf.h (1)
311-317: I2C slave-mode define — OK. Optional doc clarity.If you want to avoid confusion between “support” and “enable”, consider rewording:
- * @brief Enables the I2C slave subsystem. + * @brief Target supports I2C slave mode (capability flag).targets/ChibiOS/ORGPAL_PALX/nanoCLR/halconf.h (1)
311-317: I2C slave-mode capability — LGTM. Optional: centralize default.Given this macro repeats across many targets (nanoBooter and nanoCLR), consider a single default in halconf_nf.h with per-target overrides only when needed.
targets/ChibiOS/ST_NUCLEO64_F091RC/nanoCLR/halconf.h (1)
312-318: I2C_SUPPORTS_SLAVE_MODE added — LGTM.Safe default. If any F0-based boards plan slave-mode, document enabling in target config.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoBooter/halconf.h(2 hunks)targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoCLR/halconf.h(2 hunks)targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoBooter/halconf.h(3 hunks)targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoCLR/halconf.h(2 hunks)targets/ChibiOS/ORGPAL_PALTHREE/nanoBooter/halconf.h(2 hunks)targets/ChibiOS/ORGPAL_PALTHREE/nanoCLR/halconf.h(2 hunks)targets/ChibiOS/ORGPAL_PALX/nanoBooter/halconf.h(2 hunks)targets/ChibiOS/ORGPAL_PALX/nanoCLR/halconf.h(2 hunks)targets/ChibiOS/ST_NUCLEO64_F091RC/nanoBooter/halconf.h(2 hunks)targets/ChibiOS/ST_NUCLEO64_F091RC/nanoCLR/halconf.h(2 hunks)targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoBooter/halconf.h(2 hunks)targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoCLR/halconf.h(2 hunks)targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoBooter/halconf.h(2 hunks)targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoCLR/halconf.h(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ST_NUCLEO64_F091RC)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALX)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALTHREE)
- GitHub Check: nf-interpreter (Check_Code_Style)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (24)
targets/ChibiOS/ORGPAL_PALTHREE/nanoCLR/halconf.h (1)
24-24: HAL config version bump to 9.0 confirmed repo-wide
- No occurrences of
_CHIBIOS_HAL_CONF_VER_8_remain.- Every
halconf.hdefines only_CHIBIOS_HAL_CONF_VER_9_0_.- The
I2C_SUPPORTS_SLAVE_MODEmacro is present (defaulting to FALSE) in all ChibiOS and AzureRTOS HAL configs; no 9.0-required symbols are missing.targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoCLR/halconf.h (2)
24-24: HAL config version 9.0 bump — LGTM.
311-317: I2C_SUPPORTS_SLAVE_MODE addition — LGTM.targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoCLR/halconf.h (1)
24-24: HAL config version 9.0 bump — LGTM.targets/ChibiOS/ST_NUCLEO64_F091RC/nanoBooter/halconf.h (2)
24-24: HAL config version 9.0 bump — LGTM.
300-306: I2C_SUPPORTS_SLAVE_MODE — LGTM.targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoCLR/halconf.h (2)
24-24: HAL config version 9.0 bump — LGTM.
312-318: I2C slave-mode flag — LGTM. Ensure usage sites gate on this macro.Please confirm any I2C slave-mode code paths are wrapped with
#if I2C_SUPPORTS_SLAVE_MODEto prevent accidental inclusion where unsupported.targets/ChibiOS/ORGPAL_PALX/nanoBooter/halconf.h (2)
24-24: HAL config version 9.0 bump — LGTM.
300-306: I2C_SUPPORTS_SLAVE_MODE — LGTM.targets/ChibiOS/ORGPAL_PALX/nanoCLR/halconf.h (1)
24-24: HAL config version 9.0 bump — LGTM.targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoBooter/halconf.h (2)
24-24: HAL config version 9.0 bump — LGTM.
300-306: I2C_SUPPORTS_SLAVE_MODE — LGTM.targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoBooter/halconf.h (1)
24-24: All HAL config macros updated to 9.0 — no stale 8.4 macros found
- Verified that
_CHIBIOS_HAL_CONF_VER_8_4_does not appear anywhere in the tree.- Confirmed
_CHIBIOS_HAL_CONF_VER_9_0_is defined in everytargets/**/halconf.h(including ST_STM32F769I_DISCOVERY/nanoBooter).LGTM.
targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoCLR/halconf.h (2)
24-24: HAL conf version bumped to 9.0 — LGTM.Matches nanoBooter; good consistency.
313-319: I2C_SUPPORTS_SLAVE_MODE added — LGTM.Definition and placement look good and consistent with nanoBooter for this board.
targets/ChibiOS/ORGPAL_PALTHREE/nanoBooter/halconf.h (2)
24-24: HAL conf version bumped to 9.0 — LGTM.
300-306: I2C_SUPPORTS_SLAVE_MODE introduced — LGTM.Default FALSE with DOXYGEN guard matches the project pattern.
targets/ChibiOS/ST_NUCLEO64_F091RC/nanoCLR/halconf.h (1)
24-24: HAL conf version bumped to 9.0 — LGTM.targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoBooter/halconf.h (3)
24-24: HAL conf version bumped to 9.0 — LGTM.
300-306: I2C_SUPPORTS_SLAVE_MODE added — LGTM.Consistent with other targets.
556-556: EOF formatting tweak — OK.Non-functional newline/formatting update.
targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoBooter/halconf.h (2)
24-24: HAL conf version bumped to 9.0 — LGTM.
300-306: I2C_SUPPORTS_SLAVE_MODE added — LGTM.Pattern and default align with other boards.
| /** | ||
| * @brief Enables the I2C slave subsystem. | ||
| */ | ||
| #if !defined(I2C_SUPPORTS_SLAVE_MODE) || defined(__DOXYGEN__) | ||
| #define I2C_SUPPORTS_SLAVE_MODE FALSE | ||
| #endif | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New I2C_SUPPORTS_SLAVE_MODE macro — LGTM. Minor wording nit.
The macro name signals capability, but the brief says “Enables…”. Consider clarifying the brief to avoid confusion.
-/**
- * @brief Enables the I2C slave subsystem.
- */
+/**
+ * @brief Indicates whether the I2C driver supports slave mode.
+ */Also suggest a quick scan to ensure this macro isn’t redefined elsewhere (e.g., overlays) unless intentionally overridden.
🏁 Script executed:
#!/bin/bash
# Check for multiple/conflicting definitions or alternative spellings.
rg -n --no-heading $'^\\s*#\\s*define\\s+I2C_SUPPORTS_SLAVE_MODE\\b'
rg -n "I2C_.*SLAVE" -SLength of output: 9144
Clarify I2C_SUPPORTS_SLAVE_MODE doxygen brief
We searched all halconf.h files across targets and found only the default definitions—no conflicting overrides—so updating the comment is safe.
Please update in targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoBooter/halconf.h (and similarly in other target folders if you wish to keep consistency):
-/**
- * @brief Enables the I2C slave subsystem.
- */
+/**
+ * @brief Indicates whether the I2C driver supports slave mode.
+ */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @brief Enables the I2C slave subsystem. | |
| */ | |
| #if !defined(I2C_SUPPORTS_SLAVE_MODE) || defined(__DOXYGEN__) | |
| #define I2C_SUPPORTS_SLAVE_MODE FALSE | |
| #endif | |
| /** | |
| * @brief Indicates whether the I2C driver supports slave mode. | |
| */ | |
| #if !defined(I2C_SUPPORTS_SLAVE_MODE) || defined(__DOXYGEN__) | |
| #define I2C_SUPPORTS_SLAVE_MODE FALSE | |
| #endif |
🤖 Prompt for AI Agents
In targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoBooter/halconf.h around lines 300
to 306, the Doxygen brief for I2C_SUPPORTS_SLAVE_MODE is too terse; update the
comment to clearly state that this macro enables or disables HAL support for I2C
slave mode (default FALSE here), when it should be set to TRUE (if the
board/driver needs slave functionality), and that it controls compilation of
slave-related code paths; make the same wording change in other target halconf.h
files for consistency.
josesimoes
force-pushed
the
chibios-hal-v9
branch
from
c41935a to
fc88300
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoBooter/halconf.h (1)
24-24: Double-check cross-target uniformity and I2C slave-mode toggle presence.
- Verify no target still defines CHIBIOS_HAL_CONF_VER_8_x.
- Confirm the new I2C slave-mode define is declared where needed (likely in halconf_nf.h) and gated to HAL 9.0 to avoid breaking older boards if any remain.
Use the same verification script provided in the other file’s comment. I can follow up with a small patch to add the define across overlays once the correct macro name is verified.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoBooter/halconf.h(1 hunks)targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoCLR/halconf.h(1 hunks)targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoBooter/halconf.h(2 hunks)targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoCLR/halconf.h(1 hunks)targets/ChibiOS/ORGPAL_PALTHREE/nanoBooter/halconf.h(1 hunks)targets/ChibiOS/ORGPAL_PALTHREE/nanoCLR/halconf.h(1 hunks)targets/ChibiOS/ORGPAL_PALX/nanoBooter/halconf.h(1 hunks)targets/ChibiOS/ORGPAL_PALX/nanoCLR/halconf.h(1 hunks)targets/ChibiOS/ST_NUCLEO64_F091RC/nanoBooter/halconf.h(1 hunks)targets/ChibiOS/ST_NUCLEO64_F091RC/nanoCLR/halconf.h(1 hunks)targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoBooter/halconf.h(1 hunks)targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoCLR/halconf.h(1 hunks)targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoBooter/halconf.h(1 hunks)targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoCLR/halconf.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- targets/ChibiOS/ST_NUCLEO64_F091RC/nanoBooter/halconf.h
- targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoCLR/halconf.h
- targets/ChibiOS/ORGPAL_PALTHREE/nanoBooter/halconf.h
- targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoCLR/halconf.h
- targets/ChibiOS/ST_NUCLEO64_F091RC/nanoCLR/halconf.h
- targets/ChibiOS/ST_STM32F769I_DISCOVERY/nanoBooter/halconf.h
- targets/ChibiOS/ORGPAL_PALTHREE/nanoCLR/halconf.h
- targets/ChibiOS/ORGPAL_PALX/nanoCLR/halconf.h
- targets/ChibiOS/ORGPAL_PALX/nanoBooter/halconf.h
- targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoCLR/halconf.h
- targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoCLR/halconf.h
- targets/AzureRTOS/ST/ST_B_L475E_IOT01A/nanoBooter/halconf.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: nf-interpreter (Check_Code_Style)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ST_NUCLEO64_F091RC)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALX)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALTHREE)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (3)
targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoBooter/halconf.h (2)
24-24: HAL conf version bump to 9.0 is appropriate.This aligns with the PR objective to update to ChibiOS HAL v9.0.
24-24: Add STM32_I2C_SLAVE_ENABLE to ST-port HAL 9.0 config overlaysAll targets have been bumped to
_CHIBIOS_HAL_CONF_VER_9_0_and there are no leftover 8.x definitions. However, the STM32-specific I2C slave-enable macro isn’t defined in any of thehalconf_nf.hoverlays, so slave support won’t be compiled in.Please update each ST-port
halconf_nf.h(e.g.
targets/AzureRTOS/ST/ORGPAL_PALTHREE/nanoBooter/halconf_nf.h,
…/nanoCLR/halconf_nf.h, as well as the other ST targets) to include:+#if !defined(STM32_I2C_SLAVE_ENABLE) +#define STM32_I2C_SLAVE_ENABLE TRUE +#endifThis ensures the ChibiOS LLD conditional around
I2C_SUPPORTS_SLAVE_MODEis actually enabled. Once the macro is in place, rebuild to verify that I2C slave paths are pulled in.Likely an incorrect or invalid review comment.
targets/ChibiOS/ST_STM32F429I_DISCOVERY/nanoBooter/halconf.h (1)
24-24: HAL conf version macro updated correctly to 9.0.Change is scoped and matches the stated dependency bump.
- Following nanoframework#3202
Description
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist
Summary by CodeRabbit