Revise inverter service documentation#144
Conversation
Updated inverter service documentation to improve clarity and organization.
![indivisus[bot]](https://avatars.githubusercontent.com/in/2748893?s=60&v=4){kind=small}
| Field keys reuse the meter_elec convention documented under | ||
| [`meter_elec` → Electricity measurements](./meter.md#electricity-measurements): | ||
| `u` (voltage, V), `i` (current, A), `i_import` / `i_export` (directional | ||
| current, A), `p_import` / `p_export` (directional power, W), `freq` | ||
| (frequency, Hz), and the DC-side counterparts `dc_u`, `dc_i`, `dc_p`. |
There was a problem hiding this comment.
[P2 - Medium] Bug: Reference to non-existent field key 'i_import'
The documentation claims field keys reuse the meter_elec convention and lists i_import / i_export for directional current. However, meter.md's Electricity measurements table (lines 187-191) documents only i for current (import direction) and i_export for current export. There is no i_import key in the meter_elec convention. This creates a protocol mismatch risk where adapters implement incompatible payloads.
Evidence:
`i` (current, A), `i_import` / `i_export` (directional current, A)
Suggested fix:
Replace i_import with i to match the meter_elec convention. The correct pairing is i (import current) and i_export (export current):
`u` (voltage, V), `i` / `i_export` (directional
current, A), `p_import` / `p_export` (directional power, W), `freq`| ## Adapter guidelines | ||
|
|
||
| * `sup_pv_strings` MUST be set on the service specification so clients can discover how many PV reports to subscribe to. | ||
| * Quantities not supported by the underlying hardware (e.g. PV current on some PIP inverters) SHOULD be reported as `0` rather than omitted. |
There was a problem hiding this comment.
[P2 - Medium] Logic Error: Guideline conflates unsupported metrics with valid zero values
Recommending unsupported quantities be reported as 0 makes it impossible for consumers to distinguish "not supported" from an actual measured zero. This can silently corrupt automations, alerting, and analytics that need to know whether a value is genuinely zero or simply unavailable.
Evidence:
* Quantities not supported by the underlying hardware ... SHOULD be reported as `0` rather than omitted.
Suggested fix:
Consider one of these alternatives:
- Omit unsupported keys entirely (preferred) - consumers can check for key presence
- Use
nullfor unsupported values - explicit "not available" signal - Document in service properties which fields are supported via a
sup_fieldsarray
Suggested revision:
* Quantities not supported by the underlying hardware (e.g. PV current on some PIP inverters) SHOULD be omitted from the report rather than set to `0`.| ```json | ||
| { | ||
| "serv": "inverter", | ||
| "type": "evt.grid.report", | ||
| "val_t": "float_map", | ||
| "val": { | ||
| "u": 230.1, | ||
| "freq": 49.9, | ||
| "p_import": 0 | ||
| }, | ||
| "src": "ess", | ||
| "ver": "1" | ||
| } | ||
| ``` | ||
|
|
||
| ## Example — PV string #1 report | ||
|
|
||
| ```json | ||
| { | ||
| "serv": "inverter", | ||
| "type": "evt.pv_1.report", | ||
| "val_t": "float_map", | ||
| "val": { | ||
| "dc_u": 137.2, | ||
| "dc_i": 1.57, | ||
| "dc_p": 215 | ||
| }, | ||
| "src": "ess", | ||
| "ver": "1" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
[P3 - Low] Style: Example messages omit common FIMP fields
Both JSON examples are presented as full messages but omit fields consistently shown in peer documentation (battery.md, meter.md): props, tags, uid, and topic. While these may not be strictly required, their absence makes the examples inconsistent with other service docs and may mislead implementers.
Evidence:
Example objects include `serv`, `type`, `val_t`, `val`, `src`, `ver` only.
| ## Example — grid report | ||
|
|
||
| ```json | ||
| { | ||
| "serv": "inverter", | ||
| "type": "evt.grid.report", | ||
| "val_t": "float_map", | ||
| "val": { | ||
| "u": 230.1, | ||
| "freq": 49.9, | ||
| "p_import": 0 | ||
| }, | ||
| "src": "ess", | ||
| "ver": "1" | ||
| } | ||
| ``` | ||
|
|
||
| ## Example — PV string #1 report |
There was a problem hiding this comment.
[P4 - Info] Style: Example section header format differs from repository convention
The documentation uses separate headers with em-dashes (## Example — grid report) instead of a single ## Examples section with bullet points as used in battery.md and meter.md.
Evidence:
## Example — grid report
| | Type | Interface | Value type | Description | | ||
| |------|----------------------------|-------------|--------------------------------------------------------------------| | ||
| | in | `cmd.grid.get_report` | `null` | Request the grid report. | | ||
| | out | `evt.grid.report` | `float_map` | Grid-side measurements. Fields: `u`, `freq`, `p_import`. | | ||
| | in | `cmd.pv_<n>.get_report` | `null` | Request the PV string #n report. `n` ∈ [1, `sup_pv_strings`]. | | ||
| | out | `evt.pv_<n>.report` | `float_map` | PV string #n DC-side measurements. Fields: `dc_u`, `dc_i`, `dc_p`. | | ||
| | in | `cmd.load.get_report` | `null` | Request the load report. | | ||
| | out | `evt.load.report` | `float_map` | Load-side (inverter output) measurements. Fields: `u`, `p_import`. | | ||
| | out | `evt.error.report` | `string` | Reports processing errors. | |
There was a problem hiding this comment.
[P4 - Info] Style: Backticks in interface table differ from other documentation
The Interfaces table uses backticks around interface names and value types (e.g., cmd.grid.get_report, null). Other service docs use plain text in interface tables.
Evidence:
| in | `cmd.grid.get_report` | `null` |
1 participant
Updated inverter service documentation to improve clarity and organization.