Skip to content

Scope long-form pin Mode flags to the pin's provider - #590

Merged
bdraco merged 8 commits into
mainfrom
bugfix/pin-mode-registry-scope
Jun 3, 2026
Merged

Scope long-form pin Mode flags to the pin's provider#590
bdraco merged 8 commits into
mainfrom
bugfix/pin-mode-registry-scope

Conversation

@bdraco

@bdraco bdraco commented Jun 3, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

The long-form pin Mode checkboxes (input / output / pullup / pulldown / open_drain) were offered in full for every pin, but an external pin provider allows only a subset: an I2C expander like pca9554 permits input / output, a shift register sn74hc595 only output. The editor let users tick flags ESPHome rejects ([pullup] is an invalid option for [mode]).

This is the frontend half of #584; the backend (esphome/device-builder#1163) derives a {provider_key: [allowed_modes]} map from ESPHome's PIN_SCHEMA_REGISTRY and ships it over components/get_pin_registry_modes.

Changes

  • ESPHomeAPI.getPinRegistryModes() — typed wrapper + payload-shape filter, mirroring getIntegrationDocs.
  • src/util/pin-registry-modes-cache.ts — session-scoped cache (fetched once, shared across forms, subscribe-to-populate; a failed fetch caches {} so it doesn't retry-storm).
  • config-entry-form consumes apiContext, kicks the fetch when the context lands, and threads the map onto RenderCtx.pinRegistryModes (re-rendering when it populates).
  • config-entry-pin-renderer detects the provider key present in the pin value (pca9554) and narrows the Mode group's flag children to the allowed set before rendering. A native pin (no provider key), an unknown provider, or a missing map keeps every flag — no regression.

Verification

Unit tests cover the scoping (pca9554 hides pullup; native keeps all; unknown provider / empty map fall back). Verified end-to-end in the browser against the backend branch: with the pca9554 switch the Mode group shows only Input / Output, while the native GPIO33 binary_sensor still shows all five.

Related issue or feature (if applicable):

Types of changes

  • Bugfix (non-breaking change which fixes an issue) — bugfix
  • New feature (non-breaking change which adds functionality) — new-feature
  • Enhancement to an existing feature — enhancement
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
  • Refactor (no behaviour change) — refactor
  • Documentation only — docs
  • Maintenance / chore — maintenance
  • CI / workflow change — ci
  • Dependencies bump — dependencies

Checklist

  • The code change is tested and works locally.
  • npm run lint passes.
  • npm run test passes.
  • Tests have been added to verify that the new code works (where applicable).

A long-form pin's Mode checkboxes (input/output/pullup/pulldown/
open_drain) were offered in full for every pin, but an external provider
allows only a subset: an I2C expander like pca9554 permits input/output,
a shift register sn74hc595 only output. The editor let users tick flags
ESPHome rejects.

Fetch the backend's {provider_key: [allowed_modes]} map once per session
(components/get_pin_registry_modes, cached + shared), thread it onto
RenderCtx, and in the pin renderer detect the provider key in the pin
value and narrow the Mode group's flag children to the allowed set.
A native pin (no provider key), an unknown provider, or a missing map
keeps every flag, so there's no regression.
Copilot AI review requested due to automatic review settings June 3, 2026 19:52
@github-actions github-actions Bot added the bugfix Bug fix label Jun 3, 2026
@esphbot

esphbot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

PR Review — Scope long-form pin Mode flags to the pin's provider

Solid, well-scoped bugfix — merge-ready. All prior Copilot/Kōan review points are addressed in the current diff.

  • WS boundary is validated correctly: getPinRegistryModes() rejects non-objects, drops non-string flags, and omits providers that filter to empty so an empty allow-list can never scope the Mode group to zero checkboxes.
  • providerAllowedModes() uses Object.prototype.hasOwnProperty.call (not in), so a pin-value key like toString can't match a prototype member, and an empty list falls back to show-all.
  • Cache lifecycle is clean: fetch-once with shared in-flight promise, failure caches {} (no retry-storm) and now console.warns, listeners are isolated in try/catch, and _resetPinRegistryModesCache() clears _listeners for test isolation.
  • Subscribe/unsubscribe are balanced across connectedCallback/disconnectedCallback; the graceful fallback (undefined map / native / unknown provider / empty list) is covered by the runtime tests.
  • Conforms to the repo's lockstep policy — no backend version-skew shims; degrades to show-all when the command is absent.


Checklist

  • Input validation at boundaries (WS payload shape)
  • Error handling — failed fetch logs and doesn't retry-storm
  • No prototype-pollution on provider-key lookup (own-property check)
  • Subscription lifecycle balanced (subscribe/unsubscribe)
  • Test isolation (reset clears cache, in-flight, and listeners)
  • Edge cases covered (native / unknown provider / empty map / empty list)
  • No backend version-skew shims (lockstep policy)

Automated review by Kōan5e94836
521115d
2eb2d47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Scopes the long-form pin mode flag checkboxes in the visual config editor to the selected pin provider’s allowed modes (e.g., expander/shift-register pins), preventing users from selecting combinations ESPHome rejects.

Changes:

  • Add ESPHomeAPI.getPinRegistryModes() to fetch and sanitize the backend-provided {provider_key: allowed_modes[]} map.
  • Introduce a session-scoped cache for the pin-registry modes map and thread it through RenderCtx.
  • Narrow the rendered mode flag children in the pin renderer based on the provider key present in the pin value, with runtime tests covering provider/native/fallback behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/api/esphome-api.ts Adds getPinRegistryModes() WS wrapper with payload-shape filtering.
src/util/pin-registry-modes-cache.ts Implements a session-scoped cache + subscription mechanism for the modes map.
src/components/device/config-entry-form.ts Consumes apiContext, triggers the fetch, and passes cached modes into RenderCtx.
src/components/device/config-entry-renderers-shared.ts Extends RenderCtx with optional pinRegistryModes.
src/components/device/config-entry-pin-renderer.ts Detects provider key in pin value and scopes mode flag children accordingly.
test/util/pin-registry-modes-cache.test.ts Adds unit tests for cache memoization, subscriber notification, and failure behavior.
test/components/device/config-entry-pin-renderer-runtime.test.ts Adds runtime tests validating mode-flag scoping and graceful fallback cases.

Comment thread src/api/esphome-api.ts Outdated
Comment thread src/components/device/config-entry-pin-renderer.ts
Comment thread src/util/pin-registry-modes-cache.ts Outdated
Comment thread src/util/pin-registry-modes-cache.ts Outdated
Comment thread src/components/device/config-entry-form.ts Outdated
- getPinRegistryModes omits providers whose mode list filters to empty,
  so an empty allow-list can't scope the Mode group to zero checkboxes.
- providerAllowedModes uses an own-property check (not `in`) so a pin
  value key like `toString` can't match a prototype member, and treats
  an empty allowed list as no scoping (show every flag).
- The pin-registry-modes cache logs a failed fetch, isolates listener
  calls in try/catch, and clears listeners on reset (test isolation).
- Move the wa-select-sync JSDoc back onto _syncSelectValues; the new
  connectedCallback gets its own terse doc.
@bdraco

bdraco commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

All review points addressed in 521115d (backend #1163 is now merged):

Copilot

  1. getPinRegistryModes() now omits a provider whose flags filter to empty, so an empty allow-list can't scope the Mode group to zero checkboxes.
  2. providerAllowedModes() uses an own-property check (Object.prototype.hasOwnProperty.call, not in) so a pin-value key like toString can't match a prototype member, and treats an empty allowed list as no-scoping (show every flag).
  3. The cache now isolates each listener call in try/catch + logs.
  4. _resetPinRegistryModesCache() now clears _listeners too (test isolation).
  5. Moved the wa-select-sync JSDoc back onto _syncSelectValues; connectedCallback got its own terse doc.

Kōan

  • Suggestion Add translations #1 (reset clears listeners) and Add C++ syntax highlighting for !lambda values #2 (empty allow-list → show-all) covered by 4 and 2 above; added a unit test for the empty-list fallback.
  • Silent-failure (MEDIUM): the failed-fetch .catch now console.warns before caching {} (kept the empty-cache to avoid a per-render retry-storm, since the form kicks the fetch from updated()).

Also opened #592 to extract a shared fetch-once session-blob cache helper as a follow-up (the boilerplate pin-registry-modes-cache shares with automation-catalog-cache).

Copilot AI review requested due to automatic review settings June 3, 2026 20:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Comment thread src/components/device/config-entry-form.ts
Comment thread src/components/device/config-entry-pin-renderer.ts Outdated
Comment thread src/components/device/config-entry-pin-renderer.ts
Comment thread test/components/device/config-entry-pin-renderer-runtime.test.ts
Address review: scoping must not hide a mode flag the value already sets
(a legacy/invalid config on an expander) or the user can't untick it to
repair it. scopeModeChildren now keeps allowed flags plus any flag the
current value sets (presentModeFlags expands a scalar shorthand or reads
the object keys). Also kick the registry-modes fetch once when the api
context lands rather than on every render.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread src/api/esphome-api.ts Outdated
Comment thread src/util/pin-registry-modes-cache.ts
Build the map (and the failure-fallback empty map) with Object.create(null)
so an untrusted __proto__ / constructor key in the WS payload can't pollute
the prototype when assigned. Addresses review on #590.
@bdraco

bdraco commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

Addressed in cc2a4b3: getPinRegistryModes() now builds its result with Object.create(null) (and the cache's failure-fallback empty map too), so an untrusted __proto__ / constructor key in the WS payload can't pollute the prototype on assignment. Lookups already used Object.prototype.hasOwnProperty.call, so the map is prototype-safe end to end.

The other inline threads (own-property check, empty-list omit, listener isolation, reset clears listeners, JSDoc relocation, legacy disallowed-but-present flag visibility + its test, fetch-once) were resolved in the earlier commits 521115d / fc62120 / c393365.

Copilot AI review requested due to automatic review settings June 3, 2026 20:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@bdraco
bdraco merged commit 0423483 into main Jun 3, 2026
9 checks passed
@bdraco
bdraco deleted the bugfix/pin-mode-registry-scope branch June 3, 2026 20:34
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pin mode flag checkboxes ignore the pin registry's allowed modes (offer invalid flags for expander pins)

3 participants