feat: parse power measurements in http-power and add a power read CLI command#790
feat: parse power measurements in http-power and add a power read CLI command#790mmahut wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughHTTP Power driver's dummy ChangesHTTP Power JSON Parsing and CLI Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| url: "http://192.168.1.65/cm?cmnd=Status%2010" # Tasmota | ||
| voltage_path: "StatusSNS.ENERGY.Voltage" | ||
| current_path: "StatusSNS.ENERGY.Current" | ||
| ``` |
There was a problem hiding this comment.
Can you provide the examples for the shelly? I have one and would love to test it :)
| @base.command() | ||
| def read(): | ||
| """Read power measurements""" | ||
| for reading in self.read(): |
There was a problem hiding this comment.
for the CLI we should probably perform a single read. Otherwise this will remain in a loop reading powers without any throttling.
Another option is to provide a number of readings, and the interval between them, may be default to 1s ?
mangelajo
left a comment
mangelajo
left a comment
There was a problem hiding this comment.
just a couple of comments but looks great :) thank you so much!
| try: | ||
| return float(value) | ||
| except (TypeError, ValueError): | ||
| raise ValueError(f"value at {key!r} is not numeric: {value!r}") from None |
There was a problem hiding this comment.
The error message includes {value!r}, which dumps the full Python repr of whatever sits at the targeted path. Can a device response contain sensitive fields that should not leak into logs or client-visible error messages? If so consider including only the type name instead:
raise ValueError(f"value at {key!r} is not numeric (got {type(value).__name__})") from None| from jumpstarter.driver import Driver, export | ||
|
|
||
|
|
||
| def _json_path(data, path: str): |
There was a problem hiding this comment.
Annotations missing:
def _json_path(data: Any, path: str) -> Any:
def _extract_reading(data: Any, path: Optional[str], default_key: str) -> float:3 participants
Previously the http-power driver's
read()always returned dummy(0.0, 0.0)values.read()now parses the JSON the read endpoint returns and pulls voltage/current out of it. By default it looks for top-levelvoltage/currentkeys, but you can pointvoltage_path/current_pathat a dotted path (emeter.voltage,StatusSNS.ENERGY.Voltage,meters.0.voltage) for devices that nest them.Tested against a real Shelly Plug S Gen3.