Skip to content

Conversation

@mjs1441
Copy link
Contributor

@mjs1441 mjs1441 commented Oct 9, 2025

PICO IOConfigGPIO don't change output level when calling a second time to update direction / pullups.

Code was calling gpio_init, which set the output level to low, even when the pin was already initialised as gpio.

This was noticed when debugging a SPI device (SD card), which left the CSn low (device enabled) after initialisation,
due to starting as an input, then changing to an output.

Summary by CodeRabbit

  • Bug Fixes
    • Ensures previously unconfigured pins initialize correctly, improving reliability on PICO devices.
    • Adds clearer detection and warnings when pins are in unexpected modes to prevent misconfiguration.
    • Refines log messages to display GPIO number and encoded value for easier troubleshooting.

…e to update direction / pullups.

Code was calling gpio_init, which set the output level to low, even when the pin was already initialised to gpio mode.
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #14707 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

Refactors IOConfigGPIO in src/platform/PICO/io_pico.c to initialize the GPIO when current function is NULL, warn if the function is not SIO, and adjust logging format. Direction and pull configuration remain unchanged.

Changes

Cohort / File(s) Summary of Changes
PICO GPIO init/control flow
src/platform/PICO/io_pico.c
Reworked IOConfigGPIO: fetch current function, call gpio_init when GPIO_FUNC_NULL, warn if function not GPIO_FUNC_SIO, updated log message format. Retains gpio_set_dir and gpio_set_pulls behavior. Minor formatting/comments updated.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Caller
    participant I as IOConfigGPIO
    participant G as GPIO HAL
    participant L as Log

    C->>I: IOConfigGPIO(ioPin, ...)
    I->>G: gpio_get_function(ioPin)
    G-->>I: currentFunction

    alt currentFunction == GPIO_FUNC_NULL
        I->>G: gpio_init(ioPin)
        note right of I: Initialize when function is NULL
    else currentFunction != GPIO_FUNC_SIO
        I->>L: warn("gpio %d for 0x%02x: function=%d", ...)
        note right of I: Proceed despite non-SIO
    end

    I->>G: gpio_set_dir(ioPin, ...)
    I->>G: gpio_set_pulls(ioPin, ...)
    I-->>C: return
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

RN: BUGFIX, RPI

Suggested reviewers

  • ledvinap
  • haslinghuis
  • KarateBrot
  • blckmn

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title specifically describes preventing IOConfigGPIO from altering the output level on subsequent calls, which directly reflects the main intent of the changeset without extraneous detail or noise.
Description Check ✅ Passed The pull request description clearly explains the observed issue, the root cause, and how the code change addresses it, providing sufficient context for reviewers.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/platform/PICO/io_pico.c (1)

144-150: LGTM! Core fix correctly prevents output level reset.

The logic now properly handles re-configuration:

  • GPIO_FUNC_NULL: Initializes the pin (first-time setup)
  • GPIO_FUNC_SIO: Skips initialization, preserving the existing output level (fixes the chip-select issue)
  • Other functions: Warns but allows reconfiguration

This correctly addresses the issue where calling IOConfigGPIO a second time would unintentionally reset the output level to low.

Minor observation: Line 149's bprintf includes \n, while line 142's doesn't. Consider using consistent newline handling across log messages for uniformity.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6780892 and 73a737b.

📒 Files selected for processing (1)
  • src/platform/PICO/io_pico.c (1 hunks)
🔇 Additional comments (1)
src/platform/PICO/io_pico.c (1)

142-142: LGTM! Improved log message format.

The updated log message is more informative, using hex format for the configuration parameter and providing helpful inline documentation of the direction values.

@haslinghuis haslinghuis moved this to Bugfix in 2025.12.0 Oct 9, 2025
@haslinghuis haslinghuis added this to the 2025.12 milestone Oct 9, 2025
@haslinghuis haslinghuis added the RPI label Oct 9, 2025
@haslinghuis haslinghuis merged commit bb4c8b6 into betaflight:master Oct 15, 2025
35 checks passed
@github-project-automation github-project-automation bot moved this from Bugfix to Done in 2025.12.0 Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants