Skip to content

GPIO: always use pull-up for logic input#27321

Merged
andig merged 2 commits into
masterfrom
device/gpio_pu
Feb 11, 2026
Merged

GPIO: always use pull-up for logic input#27321
andig merged 2 commits into
masterfrom
device/gpio_pu

Conversation

@premultiply
Copy link
Copy Markdown
Member

Open (not connected to GND) => high, true, active
Closed (connected to GND) => low, false, inactive

Use the NC contact of a connected relay.

The LPC limit (§14a EnWG) is active when the relay contact is open; for normal operation, the contact must be closed. => In the event of a fault (e.g. cable break), the limit is activated (fail safe).

Copilot AI review requested due to automatic review settings February 9, 2026 19:16
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Applying p.pin.PullUp() unconditionally for all GpioTypeRead pins changes existing behavior; consider making the pull-up configuration explicit in the plugin config so callers can still choose floating or pull-down inputs where required.
  • If the underlying GPIO library distinguishes between setting pull mode before/after direction, double-check whether PullUp() should be called prior to Input() to avoid transient incorrect states during initialization.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Applying `p.pin.PullUp()` unconditionally for all `GpioTypeRead` pins changes existing behavior; consider making the pull-up configuration explicit in the plugin config so callers can still choose floating or pull-down inputs where required.
- If the underlying GPIO library distinguishes between setting pull mode before/after direction, double-check whether `PullUp()` should be called prior to `Input()` to avoid transient incorrect states during initialization.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@premultiply premultiply added devices Specific device support needs documentation Triggers issue creation in evcc-io/docs labels Feb 9, 2026
@premultiply premultiply requested a review from andig February 9, 2026 21:01
@andig
Copy link
Copy Markdown
Member

andig commented Feb 10, 2026

Not sure here. This config is not specifically LPC but GPIO in general. Should we instead make the "active" state (i.e. lo/hi) configurable and then during setup configure the inactive pull up/down accordingly?

@premultiply
Copy link
Copy Markdown
Member Author

Should be pull-up in general for all simple logic inputs. This is a physical thing. It never gets anything worse but does not work without.
Some pins already include a fixed pull-up resistor.

Optional inverting the logic on software side is another topic.
Good idea but OT here.

@premultiply premultiply changed the title GPIO: always use pull-up for input (LPC) GPIO: always use pull-up for logic input Feb 10, 2026
@andig
Copy link
Copy Markdown
Member

andig commented Feb 10, 2026

Optional inverting the logic on software side is another topic.

Thinking about this. Current logic is high-active, so it actually fits already.

@andig andig enabled auto-merge (squash) February 10, 2026 07:54
@andig andig disabled auto-merge February 10, 2026 08:07
@andig andig enabled auto-merge (squash) February 10, 2026 08:07
@andig andig disabled auto-merge February 11, 2026 08:14
@andig andig enabled auto-merge (squash) February 11, 2026 08:14
@andig
Copy link
Copy Markdown
Member

andig commented Feb 11, 2026

Build got stuck, restarted.

@andig andig merged commit e30fa4b into master Feb 11, 2026
13 of 14 checks passed
@andig andig deleted the device/gpio_pu branch February 11, 2026 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devices Specific device support needs documentation Triggers issue creation in evcc-io/docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants