Fix crash when charge/discharge time string is malformed#3930
Conversation
- Wrap datetime.strptime in time_string_to_stamp (utils.py) with try/except ValueError so invalid time strings like '14:70:00' return None instead of crashing - Add test_charge_window_invalid_format_time and test_discharge_window_invalid_format_time tests to cover this case in test_inverter.py Agent-Logs-Url: https://github.com/springfall2008/batpred/sessions/a86d98eb-f679-4432-9da4-686d12309bdf Co-authored-by: springfall2008 <48591903+springfall2008@users.noreply.github.com>
|
@copilot what about the other call sits of time_string_to_stamp in fetch.py, don't we need to remove the try/except around those and check for None instead? |
Now that time_string_to_stamp() handles ValueError internally and returns None, the try/except (ValueError, TypeError) blocks in fetch.py are redundant. Replace them with None checks to keep the same error logging and rate-skip behaviour. Agent-Logs-Url: https://github.com/springfall2008/batpred/sessions/eb3f6ef7-2369-4d94-a670-180c93ae4bcb Co-authored-by: springfall2008 <48591903+springfall2008@users.noreply.github.com>
Done in b4661df. The two Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR hardens Predbat’s time parsing so malformed inverter-provided schedule strings (e.g. 14:70:00) don’t crash inverter status updates, and it aligns rate-loading code to treat bad times as “missing” rather than exceptional.
Changes:
- Update
time_string_to_stamp()to catch malformed time strings and returnNone. - Simplify
fetch.basic_rates()by removing redundant exception handling and relying onNonechecks. - Add inverter tests covering out-of-range minute values for charge/discharge schedule times.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/predbat/utils.py | Wraps datetime.strptime() to avoid crashing on invalid time strings (but currently only catches ValueError). |
| apps/predbat/fetch.py | Replaces try/except around time_string_to_stamp() with None checks when parsing rate windows. |
| apps/predbat/tests/test_inverter.py | Adds regression tests for invalid time values (14:70:00) in charge/discharge windows and wires them into the inverter test runner. |
Comments suppressed due to low confidence (1)
apps/predbat/utils.py:784
- The warning uses
print(), which bypasses Predbat's normal logging (self.log()/record_status) and can become noisy because many callers already log a warning whentime_string_to_stamp()returnsNone(so this can double-log). Prefer returningNonewithout printing, or route the warning through the project's logging mechanism if available.
return datetime.strptime(time_string, "%H:%M:%S")
except ValueError:
print("WARN: time_string_to_stamp: invalid time string '{}', returning None".format(time_string))
return None
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
time_string_to_stamp()raised an unhandledValueErrorwhen the inverter returned an out-of-range time like'14:70:00', crashingupdate_status()entirely instead of recovering gracefully.Changes
utils.py—time_string_to_stamp(): wrapdatetime.strptimeintry/except ValueError; returnNoneon failure with a warning print. The existingNone-guards ininverter.pyalready set safe defaults (00:00:00) and log a warning, so no further changes are needed there.fetch.py: removed the now-redundanttry/except (ValueError, TypeError)blocks aroundtime_string_to_stamp()calls in the energy rates loader, replacing them withNonechecks. Sincetime_string_to_stamp()now handlesValueErrorinternally and returnsNone, the old exception handlers would never have triggered. The error logging andcontinuebehaviour are preserved exactly.tests/test_inverter.py: addtest_charge_window_invalid_format_timeandtest_discharge_window_invalid_format_timecovering the out-of-range minutes case ('14:70:00'), complementing the existing'unknown'tests.