Read the operational mode from Python components (#99)

`frequenz-microgrid-component-graph` v0.6.1 adds an operational mode to
graph
components, but the trait default leaves every component `Unspecified` —
the
bindings have to read the mode off each Python component for it to have
any
effect. This builds against v0.6.1, translates the mode, and releases
0.5.1.

## Changes

- Bump the crate from v0.6.0 to v0.6.1.
- Translate `provides_telemetry()` / `accepts_control()` into one
`OperationalMode` in `Component::try_new`, alongside the existing
category
  translation.
- Raise the `microgrid` extra's floor to `frequenz-client-microgrid >=
0.18.4`,
the release the two methods arrived in. On 0.18.3 the mode is never
readable
  and the new tests cannot run.
- Write the release notes and bump the version to 0.5.1.

## Worth a look

- **Both flags or nothing.** A mode is named only when both are known:
`provides_telemetry() == false` fits `Inactive` and `ControlOnly` alike,
so a
  half-known mode stays `Unspecified` rather than being guessed at.
- **Missing accessors are tolerated.** The assets client has no
equivalent
methods, so their absence reads as unspecified instead of failing the
graph —
  the same tolerance the category lookup already has.
- **`getattr` and `call0` are separate steps** so a missing method reads
as
unspecified while an `AttributeError` from inside the method body
propagates.
`call_method0` cannot tell those apart, and swallowing the second would
hide a
  caller's bug and leave the component measuring.

Author: shsms

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "frequenz-microgrid-component-graph-python-bindings"
-version = "0.5.0"
+version = "0.5.1"
 edition = "2024"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
@@ -10,4 +10,4 @@ crate-type = ["cdylib"]
 
 [dependencies]
 pyo3 = "0.29.0"
-frequenz-microgrid-component-graph = "0.6.0"
+frequenz-microgrid-component-graph = "0.6.2"

Cargo.toml

@@ -2,16 +2,14 @@
 
 ## Summary
 
-<!-- Here goes a general summary of what this release is about -->
+This release lets formulas take a component's operational mode into account. A component that provides no telemetry is not used as a measurement source. It is still used to classify the meter that measures it, and is measured through that meter instead.
 
 ## Upgrading
 
-<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
+- The `microgrid` extra now needs `frequenz-client-microgrid >= 0.18.4`, up from `>= 0.18.3`. A component's operational mode is read from its `provides_telemetry()` and `accepts_control()` methods, and 0.18.3 has neither, so the feature below would do nothing there. If you pin the client yourself, move the pin to `>= 0.18.4, < 0.19`.
 
 ## New Features
 
-<!-- Here goes the main new features and examples or instructions on how to use them -->
+- Formulas now take a component's operational mode into account. A component that provides no telemetry is not used as a measurement source. It is still used to classify the meter that measures it (e.g. as a PV meter or a CHP meter), so it can still be measured through that meter.
 
-## Bug Fixes
-
-<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
+  The mode is read from the component's `provides_telemetry()` and `accepts_control()` methods. A component that does not have both methods, or does not specify both values, is treated as providing telemetry and is used exactly as before. A component built from the microgrid API carries the mode the API reports for it, so formulas can change for a site that has an inactive or control-only component.

RELEASE_NOTES.md

@@ -32,7 +32,7 @@ email = "floss@frequenz.com"
 
 [project.optional-dependencies]
 microgrid = [
-  "frequenz-client-microgrid >= 0.18.3, < 0.19",
+  "frequenz-client-microgrid >= 0.18.4, < 0.19",
 ]
 assets = [
   "frequenz-client-assets >= 0.1.0, < 0.6",

pyproject.toml

@@ -3,14 +3,19 @@
 
 use frequenz_microgrid_component_graph as cg;
 
-use pyo3::{prelude::*, types::PyAny};
+use pyo3::{
+    exceptions::{PyAttributeError, PyValueError},
+    prelude::*,
+    types::PyAny,
+};
 
 use crate::{category::category_from_python_component, utils::extract_int};
 
 /// A wrapper for the Python object representing a component.
 pub(crate) struct Component {
     pub(crate) component_id: u64,
     pub(crate) category: cg::ComponentCategory,
+    pub(crate) operational_mode: cg::OperationalMode,
     pub(crate) object: Py<PyAny>,
 }
 
@@ -22,16 +27,84 @@ impl cg::Node for Component {
     fn category(&self) -> cg::ComponentCategory {
         self.category
     }
+
+    fn operational_mode(&self) -> cg::OperationalMode {
+        self.operational_mode
+    }
+}
+
+/// Reads a component's operational mode.
+///
+/// The Python side splits the mode into two flags, `provides_telemetry()`
+/// and `accepts_control()`, each of which raises `ValueError` when the mode
+/// is unspecified. Both flags together name one `OperationalMode`.
+///
+/// A mode is only named when both flags are known. One flag alone does
+/// not name a mode: `provides_telemetry() == false` fits both `Inactive`
+/// and `ControlOnly`. So a half-known mode is reported as `Unspecified`,
+/// which the graph treats as providing telemetry -- a component that says
+/// it has no telemetry but not whether it takes control keeps measuring.
+/// The API sends the two flags together or not at all, so this is a
+/// hand-built component, and reporting a mode it did not state would be
+/// a guess.
+fn operational_mode_from_python_component(
+    object: &Bound<'_, PyAny>,
+) -> PyResult<cg::OperationalMode> {
+    let (Some(telemetry), Some(control)) = (
+        specified_flag(object, "provides_telemetry")?,
+        specified_flag(object, "accepts_control")?,
+    ) else {
+        return Ok(cg::OperationalMode::Unspecified);
+    };
+
+    Ok(match (telemetry, control) {
+        (true, true) => cg::OperationalMode::ControlAndTelemetry,
+        (true, false) => cg::OperationalMode::TelemetryOnly,
+        (false, true) => cg::OperationalMode::ControlOnly,
+        (false, false) => cg::OperationalMode::Inactive,
+    })
+}
+
+/// Calls a no-argument boolean method and reads its answer, if it has one.
+///
+/// Two ways a component can have no answer, both giving `None`:
+///
+/// * The method is missing. Not every supported component type carries
+///   one: the methods arrived in `frequenz-client-microgrid` 0.18.4, and
+///   the assets client has no equivalent. Like the category lookup, the
+///   bindings keep working with a component that does not provide them.
+/// * The method is there and raises `ValueError`, which is how a
+///   component says its mode is unspecified.
+///
+/// Any other error is passed on. The method is looked up and called in
+/// two steps on purpose, so that only a missing method is read as
+/// unspecified: an `AttributeError` raised from inside the method body is
+/// the caller's own bug, and reporting that as an unspecified mode would
+/// hide it and leave the component measuring.
+fn specified_flag(object: &Bound<'_, PyAny>, name: &str) -> PyResult<Option<bool>> {
+    let method = match object.getattr(name) {
+        Ok(method) => method,
+        Err(err) if err.is_instance_of::<PyAttributeError>(object.py()) => return Ok(None),
+        Err(err) => return Err(err),
+    };
+
+    match method.call0() {
+        Ok(value) => value.extract().map(Some),
+        Err(err) if err.is_instance_of::<PyValueError>(object.py()) => Ok(None),
+        Err(err) => Err(err),
+    }
 }
 
 impl Component {
     pub(crate) fn try_new(py: Python<'_>, object: Bound<'_, PyAny>) -> PyResult<Self> {
         let component_id = extract_int(py, object.getattr("id")?)?;
         let category = category_from_python_component(py, &object)?;
+        let operational_mode = operational_mode_from_python_component(&object)?;
 
         Ok(Component {
             component_id,
             category,
+            operational_mode,
             object: object.into(),
         })
     }

src/component.rs

@@ -3,7 +3,7 @@
 
 """Tests for the frequenz.microgrid_component_graph package."""
 
-from typing import Any
+from typing import Any, NoReturn
 
 import pytest
 from frequenz.client.common.microgrid import MicrogridId
@@ -479,3 +479,169 @@ def test_unspecified_component_type_is_rejected(
                 ComponentConnection(source=ComponentId(2), destination=ComponentId(3)),
             },
         )
+
+
+def _pv_graph_with_modes(
+    *, provides_telemetry: bool | None, accepts_control: bool | None
+) -> microgrid_component_graph.ComponentGraph[
+    Component, ComponentConnection, ComponentId
+]:
+    """Build `Grid -> Meter -> SolarInverter`, with a mode on the inverter."""
+    return microgrid_component_graph.ComponentGraph(
+        components={
+            GridConnectionPoint(
+                id=ComponentId(1),
+                microgrid_id=MicrogridId(1),
+                rated_fuse_current=100,
+            ),
+            Meter(id=ComponentId(2), microgrid_id=MicrogridId(1)),
+            SolarInverter(
+                id=ComponentId(3),
+                microgrid_id=MicrogridId(1),
+                _provides_telemetry=provides_telemetry,
+                _accepts_control=accepts_control,
+            ),
+        },
+        connections={
+            ComponentConnection(source=ComponentId(1), destination=ComponentId(2)),
+            ComponentConnection(source=ComponentId(2), destination=ComponentId(3)),
+        },
+    )
+
+
+def test_operational_mode_default_is_unspecified() -> None:
+    """Test that a component with no operational mode still provides telemetry.
+
+    Both flags are `None` on a component built without them, which is the
+    unspecified mode. It is treated as providing telemetry, so graphs that
+    never set a mode keep their formulas.
+    """
+    graph = _pv_graph_with_modes(provides_telemetry=None, accepts_control=None)
+    assert graph.pv_formula(None) == "COALESCE(#3, #2, 0.0)"
+
+
+@pytest.mark.parametrize(
+    "provides_telemetry, accepts_control",
+    [
+        pytest.param(True, True, id="control-and-telemetry"),
+        pytest.param(True, False, id="telemetry-only"),
+    ],
+)
+def test_operational_mode_with_telemetry_is_a_source(
+    provides_telemetry: bool, accepts_control: bool
+) -> None:
+    """Test that a mode providing telemetry keeps the component as a source."""
+    graph = _pv_graph_with_modes(
+        provides_telemetry=provides_telemetry, accepts_control=accepts_control
+    )
+    assert graph.pv_formula(None) == "COALESCE(#3, #2, 0.0)"
+
+
+@pytest.mark.parametrize(
+    "provides_telemetry, accepts_control",
+    [
+        pytest.param(False, True, id="control-only"),
+        pytest.param(False, False, id="inactive"),
+    ],
+)
+def test_operational_mode_without_telemetry_is_not_a_source(
+    provides_telemetry: bool, accepts_control: bool
+) -> None:
+    """Test that a mode providing no telemetry drops the component as a source.
+
+    The inverter's own reading is gone; the meter above it measures it
+    instead, and still counts as a PV meter because of it.
+    """
+    graph = _pv_graph_with_modes(
+        provides_telemetry=provides_telemetry, accepts_control=accepts_control
+    )
+    assert graph.pv_formula(None) == "COALESCE(#2, 0.0)"
+
+
+@pytest.mark.parametrize(
+    "provides_telemetry, accepts_control",
+    [
+        pytest.param(False, None, id="control-unknown"),
+        pytest.param(None, False, id="telemetry-unknown"),
+    ],
+)
+def test_operational_mode_half_known_is_unspecified(
+    provides_telemetry: bool | None, accepts_control: bool | None
+) -> None:
+    """Test that a half-known mode is not guessed at.
+
+    A component can carry one flag without the other. Naming a mode from
+    that would mean guessing the missing half, so the mode is unspecified
+    and the component stays a measurement source -- even where the known
+    flag is `_provides_telemetry=False`.
+    """
+    graph = _pv_graph_with_modes(
+        provides_telemetry=provides_telemetry, accepts_control=accepts_control
+    )
+    assert graph.pv_formula(None) == "COALESCE(#3, #2, 0.0)"
+
+
+def test_operational_mode_missing_accessors_is_unspecified(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Test that a component without the mode accessors is still accepted.
+
+    `provides_telemetry()` and `accepts_control()` arrived in
+    frequenz-client-microgrid 0.18.4, and the assets client has no
+    equivalent, so a supported component can carry neither. Such a
+    component has an unspecified mode and stays a measurement source,
+    rather than failing the graph. This mirrors the category lookup, which
+    keeps working when a class is not present.
+
+    The flags below say "no telemetry", so the two paths give different
+    formulas and this test can tell them apart: only removing the methods
+    leaves `#3` in the formula. If the removal ever stopped matching, the
+    mode would read as `Inactive`, `#3` would drop out, and the test would
+    fail rather than pass while checking nothing.
+    """
+    monkeypatch.delattr(Component, "provides_telemetry")
+    monkeypatch.delattr(Component, "accepts_control")
+
+    graph = _pv_graph_with_modes(provides_telemetry=False, accepts_control=False)
+    assert graph.pv_formula(None) == "COALESCE(#3, #2, 0.0)"
+
+
+def test_operational_mode_error_inside_accessor_is_not_hidden() -> None:
+    """Test that an `AttributeError` from inside an accessor is passed on.
+
+    A missing accessor means "mode unspecified". An accessor that is
+    present but raises `AttributeError` from its own body is the caller's
+    bug: reading that as an unspecified mode would hide it and leave the
+    component measuring, which is the very thing the mode is meant to
+    stop. The lookup and the call are therefore separate steps, so that a
+    missing method reads as unspecified while an error out of the method
+    body does not.
+    """
+
+    class BrokenMeter(Meter):
+        """A meter whose accessor raises, standing in for a caller bug."""
+
+        def provides_telemetry(self) -> NoReturn:
+            """Raise, as a buggy override would.
+
+            Raises:
+                AttributeError: always.
+            """
+            raise AttributeError("nested attribute missing")
+
+    with pytest.raises(AttributeError):
+        microgrid_component_graph.ComponentGraph(
+            components={
+                GridConnectionPoint(
+                    id=ComponentId(1),
+                    microgrid_id=MicrogridId(1),
+                    rated_fuse_current=100,
+                ),
+                BrokenMeter(id=ComponentId(2), microgrid_id=MicrogridId(1)),
+                SolarInverter(id=ComponentId(3), microgrid_id=MicrogridId(1)),
+            },
+            connections={
+                ComponentConnection(source=ComponentId(1), destination=ComponentId(2)),
+                ComponentConnection(source=ComponentId(2), destination=ComponentId(3)),
+            },
+        )