Introduce the BatteryPool, and implement power bounds calculation for the pool#32
Conversation
|
Based on #31 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a high-level BatteryPool API and supporting telemetry/bounds trackers to compute aggregated dispatch power bounds for a pool of batteries (grouped by inverter↔battery topology), along with a refactor of formula streaming APIs to use metric generics.
Changes:
- Add battery pool snapshot tracking (healthy/unhealthy partitioning with latest telemetry) and bounds aggregation across inverter-battery groups.
- Expose
Microgrid/BatteryPoolas public high-level APIs and add a bounds example. - Refactor
LogicalMeterHandleformula constructors to take metrics as generic parameters (and adjust metric/formula plumbing accordingly).
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/quantity.rs | Extend Quantity with min/max constants and float-like ops used by bounds logic. |
| src/bounds.rs | Add bounds set operations (add/intersect/squash) and PbBounds conversions. |
| src/microgrid/battery_pool.rs | New BatteryPool API exposing pool power and power-bounds streams. |
| src/microgrid/battery_bounds_tracker.rs | Aggregate bounds from telemetry snapshots across groups (parallel/series topology). |
| src/microgrid/telemetry_tracker.rs | New module root for telemetry trackers. |
| src/microgrid/telemetry_tracker/battery_pool_snapshot_tracker.rs | Build inverter-battery groups from the graph; broadcast pool snapshots. |
| src/microgrid/telemetry_tracker/inverter_battery_group_telemetry_tracker.rs | Track telemetry/health for each inverter-battery group and emit group status. |
| src/microgrid/telemetry_tracker/component_telemetry_tracker.rs | Classify a component stream as healthy/unhealthy based on state codes and freshness. |
| src/microgrid.rs | Introduce Microgrid high-level entry point and wire up battery pool creation. |
| src/metric.rs | Add metric string name helper and new DcPower metric. |
| src/logical_meter/logical_meter_handle.rs | Switch formula constructors to generic metrics and expose graph() accessor. |
| src/logical_meter/logical_meter_actor.rs | Update proto import paths. |
| src/logical_meter/formula/graph_formula_provider.rs | Remove metric value plumbing from formula params; rely on metric type instead. |
| src/logical_meter/formula/coalesce_formula.rs | Replace stored metric value with PhantomData. |
| src/logical_meter/formula/aggregation_formula.rs | Replace stored metric value with PhantomData. |
| src/logical_meter/formula.rs | Update FormulaParams to remove metric value and use PhantomData. |
| src/logical_meter/config.rs | Update proto import path. |
| src/lib.rs | Export new Microgrid and BatteryPool public API. |
| src/error.rs | Add ComponentDataError and conversion from component-graph errors. |
| src/client/test_utils.rs | Extend mocks with state overrides and silent streams for timeout testing. |
| src/client/proto/electrical_component.rs | Import path cleanups. |
| src/client/microgrid_client_handle.rs | Import path cleanups in implementation and tests. |
| src/client/microgrid_client_actor.rs | Import path cleanups and request type path fix. |
| src/client/microgrid_api_client.rs | Import path cleanups for generated client type. |
| src/client/instruction.rs | Import path cleanups. |
| src/client.rs | Make client::proto public. |
| examples/logical_meter.rs | Update example to new generic-metric formula API. |
| examples/bounds.rs | New example demonstrating battery pool power bounds streaming. |
| RELEASE_NOTES.md | Document the LogicalMeterHandle API change to generic metric arguments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shsms
force-pushed
the
pool
branch
5 times, most recently
from
a4b8447 to
e2a3f27
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
This returns `BatteryPoolSnapshot` instances with the latest telemetry from all batteries and attached inverters, grouped by whether they're healthy or not. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
This way, they don't have to be specified from the CLI each time. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
| /// Returns a receiver for a stream of [`BatteryPoolSnapshot`] values, | ||
| /// each reflecting the latest component telemetry partitioned into | ||
| /// healthy and unhealthy sets. | ||
| /// | ||
| /// Reuses the running tracker if one exists and still has active receivers |
There was a problem hiding this comment.
This reuse condition means a late subscriber can keep the existing tracker alive indefinitely. In practice the task lifetime becomes "someone still has a receiver" rather than "the original caller still cares". That may be fine, but it would be good to call it out explicitly in the API/docs so the lifecycle is clear.
There was a problem hiding this comment.
Yes, but I'd say it is irrelevant to the user of the high level API. Now that I think about it, the Reuses* line is also too much info. That was not me, that was the AI. But will cleanup later.
| match battery_status { | ||
| ComponentHealthStatus::Healthy(component_id, data) => { | ||
| healthy_batteries.insert(component_id, data); | ||
| unhealthy_batteries.remove(&component_id); | ||
| } | ||
| ComponentHealthStatus::Unhealthy(component_id, data) => { | ||
| unhealthy_batteries.insert(component_id, data); | ||
| healthy_batteries.remove(&component_id); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| if let Err(e) = self | ||
| .status_tx |
There was a problem hiding this comment.
This tracker emits a full group snapshot on every single component update, and the debounce lives one layer up in BatteryPoolTelemetryTracker. That works, but it spreads the emission policy across two layers. Consider either debouncing here, or extracting a small reusable debounce/coalesce helper so the "when do we emit?" logic lives in one place.
There was a problem hiding this comment.
This is true, I'll simplify in the next PR.
This PR introduces a high-level
BatteryPoolAPI and supporting telemetry/bounds trackers to compute aggregated dispatch power bounds for a pool of batteries (grouped by inverter↔battery topology), along with a refactor of formula streaming APIs to use metric generics.Changes:
Microgrid/BatteryPoolas public high-level APIs and add a bounds example.