-
Notifications
You must be signed in to change notification settings - Fork 4
Read the operational mode from Python components #99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small warning. I changed this in client-common to use |
||
| 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") | ||
|
llucax marked this conversation as resolved.
|
||
|
|
||
| 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)), | ||
| }, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated, but is this really optional? Why? What is it used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is imported from the rust code, for checking the component category etc. Once it has moved to common, we'll just depend on common, and it becomes available for both assets and microgrid.
Right now it is optional because it is used in the tests, and it should be usable with just assets, without microgrid.
When we have common, we can make it a full dependency.