extra meter and virtual counter - #3921
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
An existing virtual-counter test still patches the removed lookup function and will fail during setup.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Extends virtual-counter calculations to support consumers with assigned extra meters.
Changes:
- Adds type-aware hierarchy object lookup.
- Includes consumer states while preventing extra-meter double counting.
- Updates and expands counter calculation tests.
File summaries
| File | Description |
|---|---|
component_parser.py |
Adds hierarchy-aware object resolution. |
_counter.py |
Handles consumers and assigned extra meters. |
_counter_test.py |
Updates mocks and tests extra-meter behavior. |
_consumer.py |
Reads consumer values from an assigned counter. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| from modules.common.store._api import LoggingValueStore | ||
| from modules.common.store._broker import pub_to_broker | ||
| from modules.common.utils.component_parser import get_component_obj_by_id | ||
| from modules.common.utils.component_parser import get_hierarchy_obj_by_id |
There was a problem hiding this comment.
🔵 Needs a closer look
get_hierarchy_obj_by_id can currently return None for unresolved but valid lookups, which can cause hard-to-debug runtime failures instead of raising a clear error at the source.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
packages/modules/common/utils/component_parser.py:79
get_hierarchy_obj_by_idreturnsNoneimplicitly when a validelement_typeis provided but the object can’t be found (e.g., missing cp/consumer entry or no component with that id). That can surface later asAttributeErrorfar from the root cause; it should raise a clear exception whenobjremainsNone.
packages/modules/common/store/_consumer.py:51- The variables
consumer/consumer_stateare misleading here because the lookup is for a counter (extra meter) object. Renaming them avoids confusion when reading and debugging this path.
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
No description provided.