Conversation
There was a problem hiding this comment.
Pull request overview
Bumps the broker/datastore schema version and adds a new datastore migration intended to fix the stored type of time_charged in chargepoint set/log data.
Changes:
- Increment
DATASTORE_VERSIONfrom 107 to 108. - Add
upgrade_datastore_108()to migrateopenWB/chargepoint/<id>/set/logpayloads wheretime_chargedis stored as a string.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| log_data = decode_payload(payload) | ||
| if log_data.get("time_charged") is not None: | ||
| if isinstance(log_data["time_charged"], str): | ||
| log_data["time_charged"] = 0 |
There was a problem hiding this comment.
time_charged in chargepoint set/log payload is defined as numeric seconds (see control/chargepoint/chargepoint_data.py Log.time_charged: float). This migration currently replaces any string value with 0, which discards the previously accumulated charging duration and can reset in-progress statistics after an update/restart. Consider converting known string formats instead (e.g. numeric strings via float(...), or H:MM / D:H:MM into seconds) and only falling back to 0.0 when parsing fails.
| log_data["time_charged"] = 0 | |
| raw_value = log_data["time_charged"] | |
| seconds = 0.0 | |
| if raw_value is not None and raw_value != "": | |
| # Try simple numeric representation first | |
| try: | |
| seconds = float(raw_value) | |
| except (TypeError, ValueError): | |
| # Try H:MM format (hours:minutes) | |
| match_h_mm = re.fullmatch(r"(\d+):(\d+)", raw_value) | |
| if match_h_mm is not None: | |
| hours_str, minutes_str = match_h_mm.groups() | |
| try: | |
| hours = int(hours_str) | |
| minutes = int(minutes_str) | |
| seconds = float((hours * 60 + minutes) * 60) | |
| except ValueError: | |
| seconds = 0.0 | |
| else: | |
| # Try D:H:MM format (days:hours:minutes) | |
| match_d_h_mm = re.fullmatch(r"(\d+):(\d+):(\d+)", raw_value) | |
| if match_d_h_mm is not None: | |
| days_str, hours_str, minutes_str = match_d_h_mm.groups() | |
| try: | |
| days = int(days_str) | |
| hours = int(hours_str) | |
| minutes = int(minutes_str) | |
| total_hours = days * 24 + hours | |
| seconds = float((total_hours * 60 + minutes) * 60) | |
| except ValueError: | |
| seconds = 0.0 | |
| log_data["time_charged"] = seconds |
| def upgrade(topic: str, payload) -> None: | ||
| if re.search("openWB/chargepoint/[0-9]+/set/log$", topic) is not None: | ||
| log_data = decode_payload(payload) | ||
| if log_data.get("time_charged") is not None: | ||
| if isinstance(log_data["time_charged"], str): | ||
| log_data["time_charged"] = 0 | ||
| return {topic: log_data} |
There was a problem hiding this comment.
The inner upgrade() callback is annotated as returning None, but it conditionally returns a dict of modified topics. For consistency with most other datastore upgrades in this file (which use Optional[dict] for callbacks passed to _loop_all_received_topics) and to avoid misleading type checking, update the return annotation accordingly.
| log_data = decode_payload(payload) | ||
| if log_data.get("time_charged") is not None: | ||
| if isinstance(log_data["time_charged"], str): | ||
| log_data["time_charged"] = 0 |
There was a problem hiding this comment.
New datastore upgrade logic (upgrade_datastore_108) isn’t covered by tests. There is already packages/helpermodules/update_config_test.py covering datastore upgrades (e.g., upgrade_datastore_94), so adding a small test that feeds all_received_topics with an openWB/chargepoint/<n>/set/log payload where time_charged is a string and asserts it is converted to the expected numeric seconds would help prevent regressions.
| log_data["time_charged"] = 0 | |
| value = log_data["time_charged"] | |
| seconds = 0 | |
| if value.isdigit(): | |
| seconds = int(value) | |
| else: | |
| parts = value.split(":") | |
| try: | |
| if len(parts) == 3: | |
| hours, minutes, secs = map(int, parts) | |
| seconds = hours * 3600 + minutes * 60 + secs | |
| elif len(parts) == 2: | |
| minutes, secs = map(int, parts) | |
| seconds = minutes * 60 + secs | |
| except ValueError: | |
| seconds = 0 | |
| log_data["time_charged"] = seconds |
1 participant
No description provided.