Repository navigation
minor: close the six gaps FTW's site test patched around - #8
Merged
Merged
Conversation
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: 1419f1b862
ℹ️ 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".
| } | ||
|
|
||
| s.sendRequest(ActionStopTransaction, req) | ||
| s.sendTxRequest(ActionStopTransaction, req, nil) |
There was a problem hiding this comment.
If a charging session starts and ends while offline, both messages are queued, but the StopTransaction payload retains the local transaction ID. On reconnect, StartTransaction receives the CSMS-assigned ID, while its callback cannot update the already-deleted transaction or the queued stop payload. For example, a local transaction 1 accepted as transaction 42 is subsequently stopped as transaction 1, leaving transaction 42 open at the CSMS. Preserve a per-session ID mapping and resolve queued StopTransaction IDs after the corresponding start response.
Useful? React with 👍 / 👎.
Comment on lines
+53
to
+57
| if limit := getSemanticValue(sim, "max_charge_power") * 1000; limit > 0 && w > limit { | ||
| w = limit | ||
| } | ||
| if limit := getSemanticValue(sim, "max_discharge_power") * 1000; limit > 0 && w < -limit { | ||
| w = -limit |
There was a problem hiding this comment.
With ems_control enabled, explicitly writing zero to Sungrow's maximum charge or discharge power register disables the corresponding clamp because of the limit > 0 condition. A forced 3000 W charge therefore still draws 3000 W with max_charge_power = 0, and self-consumption can likewise discharge despite a zero discharge limit. Distinguish an absent semantic register from a present register containing zero, and enforce zero as a maximum of zero.
Useful? React with 👍 / 👎.
Comment on lines
+1392
to
+1393
| w, _ := s.computeEffectiveLocked(connectorId, now) | ||
| return w |
There was a problem hiding this comment.
For a phase-switching charger using a one-phase profile, handleGetCompositeSchedule still calls this watts-only wrapper, divides power by three phase voltages for ampere schedules, and hardcodes NumberPhases to 3 (lines 1179–1196). Consequently, a charger actually drawing 16 A on L1 reports approximately 5.3 A on three phases, so a CSMS inspecting the composite schedule receives a different per-phase load from the one now simulated. Update that caller to use computeEffectiveLocked, convert using the effective phase count, and emit phase-count changes as schedule boundaries.
Useful? React with 👍 / 👎.
This was referenced Oct 22, 2026
Merged
A Central System that requires OCPP 1.6 security profile 1 (FTW does) refused the simulated charger: it dialled without credentials, and the WebSocket dialer rejects user:password inside the URL. Move them to an Authorization header and keep the password out of the logs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A car plugged in before the charger had reached its Central System started a local session whose StartTransaction was lost: sendRequest failed and nothing kept the message. Queue StartTransaction and StopTransaction while there is no connection and send them, in order, after the next BootNotification is accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A cloud charger such as Easee accepts a charging profile at once and
changes its output a minute or two later. The simulated charger applied
every profile at once, so nothing could test how a Central System
copes with that. POST /api/simulators/{id}/config {"profile_delay_s": 60}
now holds accepted SetChargingProfile and ClearChargingProfile changes
for that long, in order. Invalid profiles are still refused at once.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The simulated charger always spread its draw evenly over three phases, also under a profile with numberPhases 1, so a site meter never saw the one-phase load a real car puts on L1. A charger that can switch phases (ConnectorSwitch3to1PhaseSupported) now draws a 1-phase profile on L1 only, at most its rated current per phase, and reports it per phase: phasePowers and per-phase currents in its state and MeterValues. The site meter takes a charger's draw per phase instead of through the site's phase factors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Register defaults are written through each register's scale, but the Sungrow ones were raw register values: an SH10RT came up rated 100 kW (5000, 0.1 kW) with 500 kW charge and discharge limits (33046/33047, 0.01 kW). Use 10 kW and 5 kW, in the register set, the global defaults and the sungrow-hybrid and residential-5kw profiles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Sungrow model stored an EMS's writes to 13049-13051 and never
applied them, so an EMS under test commanded a battery that did not
move. With POST /api/simulators/{id}/config {"ems_control": true} the
battery does what a real SH does: in forced mode (13049 = 2) it follows
13050 (170 charge, 187 discharge, 204 stop) and 13051 watts; in any
other mode it runs self-consumption against the site meter. It answers
after ems_response_ms (default 2000), stays within 33046/33047 and the
SoC limits 13057/13058, and writes the power-flow flags (13000) the Lua
driver signs the power by. Off by default: the dashboard sets the
battery by writing the registers directly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A profile now says which srcfl/device-drivers driver it emulates
(driver_id, the file stem: sungrow, sdm630, pixii, ocpp...) and carries
the model's timing in a model block, so field evidence for a driver can
later set it. command_response_s replaces the two settings this branch
added (ems_response_ms, profile_delay_s): it is how long the device
takes to act on an accepted command, the battery after an EMS write or
the charger after a charging profile. sungrow-hybrid says 2 s; applying
a profile sets it, and POST /api/simulators/{id}/config overrides it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er a queue Codex's review of #8: - A Sungrow limit register holding 0 now allows no power that way; before, 0 turned the clamp off. - GetCompositeSchedule reports the phases the charger draws on: 16 A on one phase, not 5.3 A on three. A change of phases starts a new period. - A session that starts and ends offline is stopped under the transaction id the Central System gives its queued start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
frahlg
force-pushed
the
fix/ftw-rig-gaps
branch
from
October 23, 2026 05:34
3bf2996 to
6e5b87e
Compare
…10) * fix(ocpp): one heartbeat and meter loop per connection, DropConnection Each connection to a Central System started another heartbeat, MeterValues and simulation loop, and nothing ended them but Stop. After one reconnect a charger sent every heartbeat and MeterValues twice and integrated its energy twice a second. Heartbeats and MeterValues now end with their connection; the simulation runs once for the charger's life, connected or not, since a car charges through a network outage. Two hooks for scripted runs: DropConnection(d) closes the connection as a lost network does and keeps the charger away for d, with the session going on; SetFrameLog reports every OCPP-J frame the charger reads or writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(ocpp): the car's own limits (SetCar) A car takes at most its own current per phase and charges on its own phases: a one-phase car under a three-phase order draws on L1 only, and a car that takes 10 A draws 10 A of a 16 A offer. The charger's offer (effectivePowerW) is unchanged; its draw follows the car. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(run): headless runs from a house spec, with a timeline and a record simulator run -spec house.json -port-base N -out DIR builds one house: devices by the device-drivers driver they emulate (a hybrid whose battery follows its EMS writes, OCPP chargers with their car), the site meter behind a fuse per phase, and the house load. It serves them on ports from the base, applies a timeline of events (loads, pv, soc, plug, car, offline, online, reconnect, mark, stop), and takes the same events over POST /api/run/event while it runs. It records a sample a second of what flows through each fuse, the fuse's damage and each device's state to plant.jsonl, every OCPP frame per charger, ports.json and result.json. Self-update endpoints are off in a run. The site loop read Running without the simulator's lock while a stop wrote it; it now takes the lock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(run): an outage asked for during a retry keeps its length; "meter" is reserved Codex's review of #10: - A reconnect event while the charger was already waiting to retry only moved a deadline nobody read; the retry dialled after the usual delay. Each dial now waits out DropConnection's deadline first. - A device called "meter" lost its ports.json entry to the site meter. The spec now refuses that id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(run): draw port bases below the ephemeral range freeBase took a base from the kernel's next ephemeral port and skipped those over 65000. macOS hands ephemeral ports out in turn, so once its cursor sat near the top every try was skipped and every run test failed with "no free port range". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(run): the fuse cabinet's air and a warm start With the thermal fuse on main (#9), a run's spec can set the cabinet's air (site.fuse_ambient_c, default 25 °C) and start the fuses warm (site.fuse_preheat_a, a current per phase they have carried long enough to settle), as a house is in the evening. A fuse size the profile does not model is refused before the run touches the simulator's settings. The site loop now reads its tick before it starts: a run sets the tick before it builds its site, and the next run could set it again while the last loop was still reading it (the race detector caught it once runs followed each other within milliseconds). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * minor: faults and seeded random timelines for headless runs (#11) * feat(run): faults and seeded random timelines Faults for an EMS under test, as events in a headless run and over /api/run/event: a Modbus device that keeps its connections and answers nothing (silence), a site meter whose readings stand still (meter_late), one reading whose total disagrees with its phases (meter_glitch), phase currents that carry a sign (signed_currents), and a charger's own cap per phase (load_balancer). New model parameters, overridable per device: startup_delay_s (the car draws that long after an offer rises from zero) and load_balancer_a for an OCPP charger, signed_phase_currents for a meter. A spec's random block draws a timeline from a seed within the house's bounds (house load per phase under the fuse less 2 A), picks the open timing of each device, and writes timeline.json, a spec that replays it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(run): each session waits out the startup delay; faults keep the latest; a late meter keeps its energy Codex's review of #11: - offerSince now resets when a connector leaves its session, so every session waits out startup_delay_s, not only the first. - An earlier silence's or outage's timer no longer ends a later one: each device counts its faults and a timer acts only for the latest. - Energy that flows during meter_late shows on the counters when the readings move again; before, it was lost. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(ocpp): a charger wired on one phase wired_phases: 1 in a run spec (SetWiredPhases) wires a charger on L1. It gives an order's current there whatever numberPhases says, and max_kw is its rating as wired, so a 16 A order on a 3.68 kW charger draws 16 A on L1. Before, the order became 16 A x 3 x 230 W, was capped to 3.68 kW and spread over three phases, and a one-phase car took 5.3 A of it. FTW's site test needs it for a charger wired on one phase, under Charge now and planned charging. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FTW now tests every change to its core against this simulator: a Sungrow hybrid on Modbus, an SDM630 site meter behind an IEC gG fuse and OCPP 1.6 chargers. The first rig (21 October, outside any repo) had to patch around six gaps. This PR closes them, one commit each, each with a test, so the rig in srcfl/ftw needs no workarounds. Paired with srcfl/ftw#1797, which pins this branch's commit (1419f1b).
user:pass@in a URLuser:passwordinocpp_urlgoes out as anAuthorizationheader; logs showxxxxxTestClientSendsBasicAuthFromURL,TestClientWithoutCredentialsSendsNoAuthTestStartTransactionWaitsForTheConnectionprofile_delay_sinPOST /api/simulators/{id}/config: Accepted at once, applied that much later, in order, like a cloud charger. Invalid profiles are still refused at onceTestProfileDelayAcceptsAtOnceAndAppliesLater,TestNoProfileDelayAppliesAtOncenumberPhases: 1ConnectorSwitch3to1PhaseSupporteddraws a 1-phase profile on L1, at most its rated current. It reportsphasePowersand per-phase currents, and the site meter takes chargers per phase instead of through the phase factorsTestOnePhaseProfileDrawsOnL1,TestOnePhaseDrawIsCappedAtRatedCurrent,TestSiteMeterPutsOnePhaseChargerOnL1sungrow-hybridandresidential-5kwprofilesTestSungrowHybridDefaultsAreAnSH10RT(fails onmain)ems_control: truemakes the battery do what a real SH does: forced mode (13049 = 2) follows 13050/13051, any other mode runs self-consumption against the site meter. It answers after the model'scommand_response_s(2 s forsungrow-hybrid), keeps 33046/33047 and the SoC limits 13057/13058, and writes the power-flow flags (13000) the Lua driver signs byTestEMSForcedModeDrivesTheBattery,TestEMSSelfConsumptionCancelsTheMeter,TestEMSKeepsTheSoCLimits,TestEMSResponseTime,TestEMSWritesIgnoredWithoutEMSControlThe split: what this repo has and what FTW's rig added
The house lives here; srcfl/ftw keeps only its scenario pack (which cases, FTW's config, criteria read from FTW's API), the pinned simulator commit,
make siterigand a runner on its own machine. This is the first of three PRs; #9 and the headless-run PR follow.scenarios.go), weather scenarios, generatorsruntime.go)fuse.go)mainfuse(#9), for FTW'ssitesimto importems_control+command_response_sherecommand_response_shereSetCar)reconnect)stack.py,rig.pysimulator run -spec)plant.jsonl)ocpp-<id>.jsonl)RIG_PORT_OFFSET-port-base,/api/run/event)Models and the driver each emulates
driver_idis the srcfl/device-drivers driver a model emulates, by file name without.lua;ocppis FTW's own OCPP Central System. Timing and quirks live in each model'smodelblock, so field evidence for a driver can set them later.driver_idmodelsungrow-hybridsungrowcommand_response_s: 2solis-hybridsolissolaredgesolaredgesma-tripowersmahuawei-sun2000huaweifronius-gen24froniusdeye-hybriddeyepixii-powershaperpixiiferroamp-energyhubferroamp_modbussdm630sdm630fronius-smart-meterfronius_smart_meterambiboxambibox_v2xocpp-charger,chargeamps-dawnocppresidential-5kw,commercial-50kw,defaultcommand_response_sreplaced the two settings this branch first added (ems_response_ms,profile_delay_s).What changes for other users
ems_controlis off unless set, andcommand_response_sis 0 except where a model says otherwise. The dashboard still sets the battery by writing the registers.openapi.jsonand the MCP tool for/api/simulators/{id}/config.go test -race ./...passes.Overlaps
physicalscenario. This PR keeps to the existing aggregation path and an opt-in flag, so the two can land in either order.Not in this PR
Review fixes (Codex, 1419f1b)
TestEMSKeepsZeroPowerLimitsGetCompositeSchedulereported a one-phase draw as a third of it on three phasesTestCompositeScheduleKeepsThePhasesTestQueuedStopNamesTheAssignedTransactionRebased on
mainwith #9 (the thermal fuse) on 23 October. Nothing in this PR changed; #10 and #11 follow on top.🤖 Generated with Claude Code