]> Piment Noir Git Repositories - e-mobility-charging-stations-simulator.git/commit
refactor: audit-driven cleanup — OCPP conformance fixes, physics, harmonization ...
authorJérôme Benoit <jerome.benoit@piment-noir.org>
Mon, 6 Jul 2026 16:09:30 +0000 (18:09 +0200)
committerGitHub <noreply@github.com>
Mon, 6 Jul 2026 16:09:30 +0000 (18:09 +0200)
commitf8d8e03f399f98fdbfec4716c555f259b39ab5ec
treef831426c39dcf2575a92e18016c444b9b7aadb06
parent22cb25f02648f1efcccdf05db595f33862b4b709
refactor: audit-driven cleanup — OCPP conformance fixes, physics, harmonization (#1960)

* fix(ocpp/1.6): correct GetDiagnostics log directory resolution

`handleRequestGetDiagnostics` called `resolve()` with the comma operator
by accident:

  resolve((fileURLToPath(import.meta.url), '../', dirname(logFile)))

The extra parentheses wrap a comma-operator expression that evaluates to
`dirname(logFile)` only, so `readdirSync` was invoked with a
directory resolved from CWD instead of relative to the module URL. When
the process CWD is not the project root (e.g. a systemd service, a docker
container that mounts logs elsewhere, a background worker with a chdir'd
parent), the log-file discovery scanned the wrong directory and produced
an incomplete diagnostics archive.

Align with the sibling call at line 1183 which already uses the intended
pattern:

  resolve(dirname(fileURLToPath(import.meta.url)), '../', diagnosticsArchive)

Both call sites now resolve paths relative to the module's parent
directory. Gates pass (typecheck / lint).

* fix(physics): use sqrt(3) for line-to-line voltage derivation

Two sites derived the line-to-line nominal voltage from Math.sqrt(numberOfPhases) * V_LN:

- src/charging-station/ocpp/OCPPServiceUtils.ts:1255
- src/charging-station/meter-values/CoherentMeterValueBuilder.ts:282

The ratio V_LL / V_LN in a balanced 3-phase Y system is a fixed sqrt(3),
which comes from the 30-degree phase separation between line and neutral
voltages (V_LL = 2 * V_LN * sin(60 deg) = sqrt(3) * V_LN). It is NOT a
function of the phase count.

Both call sites are currently reachable only when numberOfPhases === 3
(one guarded explicitly at CoherentMeterValueBuilder.ts:281; the other
implicitly via the phaseLineToLineVoltageMeterValues + phase-rotation
plumbing that only supports 3-phase). Numerically the emitted value is
unchanged, but the formula was a trap: any future relaxation of the
3-phase guard would silently emit sqrt(2) * V_LN at N=2, which is
physically meaningless.

Replace both call sites with Math.sqrt(3) and add a code comment stating
the physics derivation so the constant cannot be re-parameterized on the
phase count.

Gates pass (typecheck / test, 2955 pass / 0 fail).

* refactor(auth): standardize on OCPP 2.0.1 terminology in auth subsystem

The recently-added auth subsystem used bare 'OCPP 2.0' throughout its
comments and JSDoc while the rest of the codebase (README, AGENTS.md,
all other source files) uses 'OCPP 2.0.1' as the canonical spec version
and 'OCPP 2.0.x' as the family. The auth subsystem also used non-existent
forms 'OCPP 2.0+' and 'OCPP 2.0/2.1' that misrepresented the supported
spec range: OCPP 2.1 is not implemented.

Bulk-replace across src/charging-station/ocpp/auth/:
- 'OCPP 2.0+'   -> 'OCPP 2.0.1'
- 'OCPP 2.0/2.1' -> 'OCPP 2.0.1'
- bare 'OCPP 2.0' -> 'OCPP 2.0.1'  (negative lookahead on '.digit' so
  existing 'OCPP 2.0.1' and 'OCPP 2.0.x' occurrences are preserved)

65 occurrences updated across 5 files. Files touched:
- adapters/OCPP20AuthAdapter.ts   (31)
- types/AuthTypes.ts              (21)
- strategies/CertificateAuthStrategy.ts (7)
- utils/AuthValidators.ts         (5)
- strategies/RemoteAuthStrategy.ts (1)

Zero code behavior change; comment/JSDoc content only.
Gates pass (typecheck / lint).

* refactor(ocpp/1.6): harmonize 'Central System' capitalization for logs and docs

Two mismatches surfaced against OCPP 2.0 counterparts:

1. `csmsName` field values in OCPP16IncomingRequestService.ts:176 and
   OCPP16ResponseService.ts:129 held the lowercase two-word form
   'central system'. The abstract base classes interpolate this field
   into runtime error messages, producing 'not registered on the central
   system' for OCPP 1.6 versus 'not registered on the CSMS' for OCPP
   2.0. Two spec-correct forms are involved (OCPP 1.6 uses 'Central
   System'; OCPP 2.0.1 uses 'CSMS' for 'Charging Station Management
   System'), so keep the spec-correct term per version but capitalize
   the 1.6 value to match OCPP 1.6 spec convention.

2. JSDoc prose across OCPP 1.6 files ('sends X to the central system',
   'response from the central system', etc.) mixed the lowercase form
   with the capitalized 'Central System (CS)' used in the module-level
   docstring of OCPP16IncomingRequestService.ts. Normalize to the
   capitalized form throughout src/charging-station/ocpp/1.6/.

Zero code behavior change; only logged strings and comment prose shift
casing. Gates pass (typecheck / lint).

* refactor: apply numeric-literal underscore separators to 5-digit-plus constants

Numeric-literal style was inconsistent: OCPP20Constants.ts uses
underscore separators for readability (e.g. `30_000`, `5_000`),
while src/utils/Constants.ts and other constants files used bare
literals (`60000`, `30000`, `1048576`).

Apply underscore separators uniformly to 5+ digit numeric literals
across the constants files, matching the OCPP20Constants.ts precedent:

- src/utils/Constants.ts: `60000` -> `60_000` (6 sites),
  `30000` -> `30_000`, `281474976710655` -> `281_474_976_710_655`,
  `2147483647` -> `2_147_483_647`.
- src/charging-station/ocpp/OCPPConstants.ts:
  `OCPP_WEBSOCKET_TIMEOUT_MS = 60000` -> `60_000`.
- src/charging-station/ui-server/UIServerSecurity.ts:
  `DEFAULT_MAX_PAYLOAD_SIZE_BYTES = 1048576` -> `1_048_576`,
  `DEFAULT_RATE_WINDOW_MS = 60000` -> `60_000`,
  `DEFAULT_MAX_TRACKED_IPS = 10000` -> `10_000`.

4-digit values (1000, 3600, 5000, 8080) are intentionally left unchanged;
they include port literals (`DEFAULT_UI_SERVER_PORT = 8080`) where the
underscore form is unusual. AGENTS.md numeric-literal style is a
readability convention rather than an enforced rule; applying it only
above the 5-digit threshold matches the existing OCPP20Constants.ts
pattern.

Zero code behavior change. Gates pass (typecheck / lint / test, 2955
pass / 0 fail).

* chore: expose WILDCARD_HOSTS + document Authorize in OCPP 2.0.x README table

Two low-risk harmonization findings from the from-zero audit.

1. README OCPP 2.0.x commands table missing 'Authorize' (Lane 5 M2)
   The 'Authorize' command is fully implemented for OCPP 2.0.x:
   - OCPP20RequestCommand.AUTHORIZE registered
   - OCPP20RequestService.ts / OCPP20ResponseService.ts handlers wired
   - OCPP20ServiceUtils.ts sending helper
   but the README '#### C. Authorization' section listed only 'ClearCache'.
   Add ':white_check_mark: Authorize' alongside 'ClearCache' so the docs
   match the actual implementation surface.

2. Extract WILDCARD_HOSTS set (Lane 4 M20)
   AbstractUIServer.ts:1325 inlined the three literals '', '0.0.0.0', '::'
   as a triple '===' chain to detect wildcard host configuration, while
   UIServerAccessPolicy.ts:23 already held the same three values in a
   module-scoped 'WILDCARD_HOSTS' set. Two independent definitions of
   the same domain concept.
   Export WILDCARD_HOSTS (typed 'ReadonlySet<string>') from
   UIServerAccessPolicy.ts, import it in AbstractUIServer.ts, and
   replace the inline OR chain with 'WILDCARD_HOSTS.has(configuredHost)'.

Zero behavior change. Gates pass (typecheck / lint).

* fix(ocpp/1.6): return Accepted for DataTransfer with matching vendor and messageId

Per OCPP 1.6 §4.3, the DataTransfer response statuses are:
- `UnknownVendorId` when the vendorId is not recognized
- `UnknownMessageId` when the vendorId is recognized but the messageId
  is not
- `Accepted` when both vendor and message are recognized

`handleRequestDataTransfer` previously returned `UnknownMessageId` for
*any* non-null messageId, even when the vendorId matched the station's
configured `chargePointVendor`:

    if (vendorId !== chargingStation.stationInfo?.chargePointVendor) {
      return ... UNKNOWN_VENDOR_ID
    }
    if (messageId != null) {
      return ... UNKNOWN_MESSAGE_ID     // wrong: any non-null messageId
    }

That rejected every legitimate DataTransfer with a messageId, regardless
of whether the messageId would have been supported. The simulator does
not maintain a per-vendor messageId registry (it has no way to know
which custom messageIds a real charge point would accept), so the
spec-correct behaviour is to accept any messageId once the vendor
matches.

Drop the erroneous `messageId != null` branch and return `Accepted`
whenever the vendor matches; the `UnknownVendorId` path is unchanged.
Add a source comment citing OCPP 1.6 §4.3 to prevent regression of the
buggy check.

Test update: the existing 'should return UnknownMessageId for matching
vendor with messageId' test asserted the old buggy behaviour. Rewritten
as 'should return Accepted for matching vendor with messageId (no
messageId registry per §4.3)' to lock in the spec-correct outcome.

Gates pass (typecheck / test, 2955 pass / 0 fail).

* fix(ocpp/1.6): emit FirmwareStatusNotification(Installed) after Installing

Per OCPP 1.6 §4.5, the Charge Point SHALL notify the CSMS of firmware
update progress via FirmwareStatusNotification. `updateFirmwareSimulation`
already emitted the intermediate statuses (Downloading -> Downloaded ->
Installing) but never signalled successful completion — after emitting
`Installing`, the simulation went straight to the optional reset step
(if configured) or returned, leaving the CSMS with no confirmation
that installation actually finished.

Insert an `Installed` status emission between the InstallationFailed
early-return branch and the optional reset:

    sleep(...)
    requestHandler(FIRMWARE_STATUS_NOTIFICATION, { status: Installed })
    stationInfo.firmwareStatus = Installed
    if (reset === true) { sleep(...); reset() }

Behaviour on the failure path is unchanged: the InstallationFailed
branch still returns early without emitting `Installed`. Behaviour
when `reset` is disabled is unchanged apart from the additional
notification (which is the whole point of the fix).

Gates pass (typecheck / test, 2935 pass / 0 fail).

* fix(ocpp/1.6): return Failed (not NotSupported) when LocalAuthList is disabled or manager missing

Per OCPP 1.6 §5.15, the SendLocalList response statuses have distinct
semantics:
- NotSupported: the LocalAuthListManagement feature profile is not
  implemented by the Charge Point
- Failed: the feature is implemented but the specific update failed
  (disabled, unavailable, or otherwise unable to apply the change)

`handleRequestSendLocalList` correctly returned NotSupported when the
LocalAuthListManagement profile is absent (line 1534). But two
subsequent guards also returned NotSupported when:
- `LocalAuthListEnabled` config is false (feature supported but disabled)
- `getLocalAuthListManager()` returns null (feature supported but manager
  not wired)

Both scenarios describe a Charge Point that supports the feature — the
LocalAuthListManagement profile is present — but cannot apply this
particular update. Per spec, that is Failed, not NotSupported.

Change both guards to return
`OCPP16Constants.OCPP_SEND_LOCAL_LIST_RESPONSE_FAILED`. The profile-
absent guard (line 1534) is unchanged — it correctly reports
NotSupported when the feature is not implemented at all.

Test updates: two existing tests locked in the old NotSupported result.
Renamed and updated to assert Failed with an inline reference to §5.15
so the spec-correct outcome is regression-locked.

Gates pass (typecheck / test, 2945 pass / 0 fail).

* chore(auth): remove OCPPAuthServiceImpl from public barrel

`OCPPAuthServiceImpl` is the concrete implementation of the
`OCPPAuthService` interface. External code should construct instances
through `OCPPAuthServiceFactory` (which internally uses the class)
rather than instantiating it directly. Exporting the concrete class
from the auth barrel published an implementation detail that:

- Adds public API surface with no consumer benefit
- Encourages callers to bypass the factory's per-station singleton
- Complicates future refactoring of the concrete class

Grep across src/ and ui/ confirms zero consumers of
`OCPPAuthServiceImpl` via the barrel (`ocpp/auth/index.js`); the
factory imports it via the direct file path within the same subtree.
Two test files (`OCPP20ResponseService-CacheUpdate.test.ts`,
`OCPP20ServiceUtils-AuthCache.test.ts`) did import it via the barrel;
routed to the direct path so they still exercise the concrete class
where explicitly needed.

Also correct the module-level JSDoc from 'OCPP 1.6 and 2.0' to
'OCPP 1.6 and 2.0.1' to match the canonical spec-version convention
used throughout the codebase.

Gates pass (typecheck / lint / test, 2955 pass / 0 fail).

* refactor: extend OCPP 2.0.1 terminology to non-auth paths and compress physics comment

Two round-3 review findings from PR #1960 self-review addressed together.

1. Round-3 High: terminology scope creep

Prior commit dbc3f3ff bulk-replaced OCPP 2.0 -> OCPP 2.0.1 in
src/charging-station/ocpp/auth/ only (65 occurrences). Identical drift
existed across the rest of the OCPP 2.0.x code paths, including the
invalid OCPP 2.0+ form that dbc3f3ff had explicitly corrected in auth/
but left intact in ~10 sites of OCPP20IncomingRequestService JSDoc.
After dbc3f3ff, auth/ said OCPP 2.0.1 while the rest of the OCPP 2.0.x
subtree still said OCPP 2.0 / OCPP 2.0+ — the terminology divergence
was worse, not better.

Apply the same three-pass perl regex (strip +, collapse /2.1,
negative-lookahead on .digit for bare 2.0) to:

- src/charging-station/ocpp/2.0/**/*.ts               (56 occurrences)
- src/charging-station/ocpp/OCPPServiceUtils.ts       (3)
- src/charging-station/ui-server/UIMCPServer.ts       (2)
- ui/common/tests/payloadBuilders.test.ts             (2)

63 total substitutions. Zero code behavior change; comment/JSDoc/test
description content only. Zero remaining bare OCPP 2.0 refs under the
scanned scope.

2. Round-3 Low: physics comment verbosity

The sqrt(3) physics anchor comment added in commit 34a574f9 spanned 8
lines with tangential explanations. Compress to 3 lines that keep the
essential physics anchor (V_LL = sqrt(3) * V_LN, 30-degree phase
separation, 3-phase-only constraint).

Documented as still-deferred:
- Narrative comment at OCPP16IncomingRequestService.ts:276 — kept under
  M5 bulk cleanup deferral.
- OCPP 2.0.x handleRequestDataTransfer UnknownVendorId design — a
  different intentional choice, not the same bug as OCPP 1.6 C3.

All gates green (format / typecheck / lint / build / test) across root,
ui/cli, ui/web. Circular-deps baseline of 61 preserved.

* chore: complete OCPP 2.0.1 terminology sweep and drop redundant physics comment line

Round-4 review findings from PR #1960 self-review addressed together.

1. Terminology sweep completion (Low)

The round-3 extension (commit e03368ff) covered
src/charging-station/ocpp/2.0/**, OCPPServiceUtils.ts, UIMCPServer.ts,
and payloadBuilders.test.ts, but the audit's terminology finding
applied to the whole codebase. Four bare 'OCPP 2.0' refs survived in
charging-station/ files outside ocpp/2.0/:

- src/charging-station/ConfigurationKeyUtils.ts:
  user-facing warning 'use OCPP 2.0 variable ... instead'
- src/charging-station/meter-values/CoherentMeterValueBuilder.ts:
  code comment 'Narrow the OCPP 2.0 SampledValueTemplate.unit ...'
- src/charging-station/TemplateSchema.ts (2 sites):
  JSDoc references to 'OCPP 2.0 unit-of-measure descriptor' and
  'OCPP 2.0 namespaced keys'

Apply the same negative-lookahead perl regex; 4 substitutions, zero
code behaviour change. The user-facing warning in ConfigurationKeyUtils
now cites the spec version consistently with the rest of the codebase.

2. Physics comment redundancy (Info)

The compressed sqrt(3) anchor comment from commit e03368ff sat
directly below a preexisting one-liner ('Line-to-line voltage is only
defined for 3-phase AC') that duplicated the L-L === 3-phase constraint
already stated in the compressed block ('Defined only for numberOfPhases
=== 3'). Delete the preexisting redundant line; the compressed anchor
carries the full physics rationale (V_LL = sqrt(3) * V_LN, 30-degree
phase separation, 3-phase-only constraint).

Test files (~15 refs of 'OCPP 2.0' as describe/it group labels) are
intentionally left as-is: those are test-grouping labels rather than
spec-version references, and renaming them would break dev-side test
regex filters without functional benefit.

Gates green (format / typecheck / lint / build / test) across root;
ui/cli and ui/web untouched. Circular-deps baseline of 61 preserved.
35 files changed:
README.md
src/charging-station/ConfigurationKeyUtils.ts
src/charging-station/TemplateSchema.ts
src/charging-station/meter-values/CoherentMeterValueBuilder.ts
src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts
src/charging-station/ocpp/1.6/OCPP16RequestService.ts
src/charging-station/ocpp/1.6/OCPP16ResponseService.ts
src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts
src/charging-station/ocpp/1.6/__testable__/index.ts
src/charging-station/ocpp/2.0/OCPP20CertificateManager.ts
src/charging-station/ocpp/2.0/OCPP20Constants.ts
src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts
src/charging-station/ocpp/2.0/OCPP20RequestBuilders.ts
src/charging-station/ocpp/2.0/OCPP20ResponseService.ts
src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts
src/charging-station/ocpp/2.0/__testable__/OCPP20RequestServiceTestable.ts
src/charging-station/ocpp/2.0/__testable__/OCPP20ResponseServiceTestable.ts
src/charging-station/ocpp/2.0/__testable__/index.ts
src/charging-station/ocpp/OCPPConstants.ts
src/charging-station/ocpp/OCPPServiceUtils.ts
src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts
src/charging-station/ocpp/auth/index.ts
src/charging-station/ocpp/auth/strategies/CertificateAuthStrategy.ts
src/charging-station/ocpp/auth/types/AuthTypes.ts
src/charging-station/ocpp/auth/utils/AuthValidators.ts
src/charging-station/ui-server/AbstractUIServer.ts
src/charging-station/ui-server/UIMCPServer.ts
src/charging-station/ui-server/UIServerAccessPolicy.ts
src/charging-station/ui-server/UIServerSecurity.ts
src/utils/Constants.ts
tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-LocalAuthList.test.ts
tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-SimpleHandlers.test.ts
tests/charging-station/ocpp/2.0/OCPP20ResponseService-CacheUpdate.test.ts
tests/charging-station/ocpp/2.0/OCPP20ServiceUtils-AuthCache.test.ts
ui/common/tests/payloadBuilders.test.ts