fix(ocpp): first UI enable works, and the save tells the truth about restarting - #1015
Conversation
…restarting Exercised #999's Chargers panel in a browser and hit two defects on the first enable: The username field rendered empty with a "ftw" placeholder, so a fresh save posted username: "" and validation rejected it — every first enable failed with a 400 the operator had no reason to expect. The field now carries the real default. Worse, after fixing the payload the save reported "no restart needed" while the listener never opened: the central system is started once in main.go and the config applier neither starts nor stops it, but RestartRequiredFor had no ocpp entry — the exact silent failure that file's own comment warns about. Saving an OCPP change now raises the restart prompt, and after a restart the port listens (verified live: enable → save → prompt names ocpp → restart → 8887 accepting). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1d4uknzs7QUv7nE9rW4fi
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e32672e45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the Chargers panel's enable toggle saved cleanly, reported no | ||
| // restart needed, and the listener never opened — the exact silent | ||
| // failure the comment at the top of this file warns about. | ||
| if !pointerEqual(oldCfg.OCPP, newCfg.OCPP) { |
There was a problem hiding this comment.
When the existing config has no ocpp block, merely visiting the Chargers tab and saving creates a disabled block because the generic capture pass writes the form's defaults (enabled: false, port, path, and username). This raw pointer comparison therefore reports restart_required even when the operator only edited a loadpoint or vehicle and OCPP remains disabled, prompting an unnecessary service restart despite no listener state changing. Treat absent and disabled OCPP configurations as equivalent, while still requiring restart when enabling, disabling, or changing an active listener.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
1 participant
Browser-exercised #999's Chargers panel end to end (enable → save → restart → charger port). Two defects surfaced on the very first enable, both fixed:
value=""with aftwplaceholder; validation (correctly) requires username+password when enabled, so every first enable failed with a 400. The field now carries the real default — placeholder promises are now values.restart_requiredlied for OCPP. The central system starts once inmain.go; the config applier neither starts nor stops it. With noocppentry inRestartRequiredFor, the save reported "no restart needed" while the listener never opened — an apparently successful save with a silently closed port, the exact failure mode the file's own header warns about. Saving an OCPP change now raises the restart modal with the reason.Verified live in a browser
Fresh config → Chargers tab → check "Run the OCPP server", type a password, Save → username auto-carries
ftw, config written, restart modal shows "ocpp — the central system listener is started at startup" → restart → port 8887 accepting connections.Tests:
TestRestartRequiredFor_BootSectionsgains the ocpp case;loadpoints.test.mjspins the username default (451 web tests green).Deliberately unchanged (follow-up issue): per-charger credentials and TLS stay config-file-only, as #999 chose.
🤖 Generated with Claude Code
https://claude.ai/code/session_01C1d4uknzs7QUv7nE9rW4fi
Note
Low Risk
Small UI default and restart-prompt wiring for OCPP; no changes to OCPP protocol handling or auth logic beyond honest operator feedback.
Overview
Fixes two Chargers-panel bugs that blocked turning on the built-in OCPP server from Settings.
The username field now submits the default
ftwas the input value when config has no username, instead of an empty value with only a placeholder — so the first enable no longer fails validation with a 400.Restart detection now treats any
ocppconfig change as requiring a restart, with the reason that the central-system listener is started at boot only. Saves that enable or change OCPP settings therefore show the restart modal instead of claiming no restart is needed while the port stays closed.Tests cover the new restart reason and the username default in the OCPP server form.
Reviewed by Cursor Bugbot for commit 9e32672. Bugbot is set up for automated code reviews on this repo. Configure here.