fix: change `vehicle_eta` to return datetime object or None #501

firstof9 · 2026-01-09T15:57:21Z

Summary by CodeRabbit

Breaking Changes
- vehicle_eta now returns an absolute UTC datetime (expected completion time) instead of a duration in seconds.
- Status payload field renamed: time_to_full_charge replaces the previous vehicle_eta key.
Tests
- Time-dependent tests updated to freeze current time and assert datetime results; added tests for ISO 8601 parsing and error fallbacks.
Chores
- Added a testing helper dependency for frozen-time tests; package version bumped.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-09T15:57:26Z

📝 Walkthrough

Walkthrough

Time handling updated: time now parses ISO 8601 (treating "Z" as UTC) to return datetime | None; vehicle_eta now returns a UTC datetime | None computed as now (UTC) plus seconds when present. Tests, fixture key, test deps, and package version bumped accordingly.

Changes

Cohort / File(s)	Summary
Core implementation `openevsehttp/__main__.py`	Replaced `datetime` import with `from datetime import datetime, timedelta, timezone`; changed `time(self) -> datetime
Test requirements `requirements_test.txt`	Added `freezegun` dependency to enable freezing time in tests.
Test fixtures `tests/fixtures/v4_json/status.json`	Renamed payload key `vehicle_eta` → `time_to_full_charge` (numeric value unchanged: `18000`).
Tests — main `tests/test_main.py`	Added datetime and `freezegun` imports; use `@freeze_time` for time-sensitive tests; updated test parameters and assertions so `test_get_time` expects ISO-derived `datetime
Tests — context formatting `tests/test_main_edge_cases.py`, `tests/test_websocket.py`	Converted multi-context `with` statements to grouped `with (patch(...), ...)` form; no behavioral changes.
Packaging `setup.py`	Bumped `VERSION` from `"0.2.1"` to `"0.2.2"`.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

chore: version bump #497 — also updates setup.py version constant (package version bump).

Suggested labels

enhancement

Poem

🐇 I nibble timestamps in the night,
Turn seconds to UTC light,
Freeze a moment, add a hop,
ETA appears—tick-tock, nonstop,
Rabbit code: neat, quick, and bright.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly and accurately describes the main API change: converting vehicle_eta from returning int \| None to datetime \| None.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 053b75c and 9c8880f.

📒 Files selected for processing (2)

setup.py
tests/test_main.py

🧰 Additional context used

🧬 Code graph analysis (1)

tests/test_main.py (1)

openevsehttp/__main__.py (4)

update (185-208)

time (1147-1156)

ws_disconnect (302-306)

vehicle_eta (1318-1325)

🔇 Additional comments (6)

tests/test_main.py (5)

11-12: LGTM!

The addition of datetime modules and freezegun is appropriate for testing time-based functionality. The freeze_time decorator is essential for deterministic testing of vehicle_eta, which depends on the current time.

492-510: LGTM!

The updated test correctly validates the new time property behavior:

Properly parses ISO 8601 timestamps with "Z" suffix to UTC datetime objects

Validates the type with isinstance check

Correctly handles None case

513-527: LGTM!

Comprehensive error handling test that validates the time property returns None for various invalid input types. This ensures defensive programming against malformed or unexpected data from the API.

1294-1314: LGTM!

The test correctly uses @freeze_time to ensure deterministic testing of the vehicle_eta property, which depends on datetime.now(timezone.utc). The calculation logic is correct: frozen time + expected_seconds should equal the returned datetime.

2275-2278: LGTM!

Clean refactoring to use Python 3.10+'s grouped context manager syntax. The logic remains unchanged while improving readability.

setup.py (1)

9-9: Version bump level cannot be verified without knowing the previous return types.

The current implementation shows time() and vehicle_eta() return datetime | None, but without access to the previous implementation or a changelog documenting what these properties previously returned, it's unclear whether this constitutes a breaking change warranting a minor version bump. Check the git history or release notes for the prior version to confirm whether the return type change occurred and whether a minor bump (0.3.0) is more appropriate than the patch bump (0.2.2).

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Pylint (4.0.4)

setup.py

tests/test_main.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @openevsehttp/__main__.py:
- Around line 1311-1316: In vehicle_eta, the current truthy check on value will
treat a time_to_full_charge of 0 as False and return 0 instead of a datetime;
change the condition to an explicit is not None check on
self._status.get("time_to_full_charge") so that when value == 0 you compute and
return datetime.now(timezone.utc) + timedelta(seconds=value), and only return
None when the key/value is actually missing or None.

🧹 Nitpick comments (1)

requirements_test.txt (1)

11-11: Consider pinning the freezegun version for reproducibility.

Since freezegun is used in tests (from freezegun import freeze_time in test_main.py), pinning its version ensures consistent test behavior across environments. Latest stable version is 1.5.5.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 031d70d and 239fcc7.

📒 Files selected for processing (4)

openevsehttp/__main__.py
requirements_test.txt
tests/fixtures/v4_json/status.json
tests/test_main.py

🧰 Additional context used

🧬 Code graph analysis (1)

tests/test_main.py (2)

tests/test_main_edge_cases.py (1)

charger (16-17)

openevsehttp/__main__.py (2)

update (185-208)

vehicle_eta (1311-1316)

🔇 Additional comments (5)

tests/fixtures/v4_json/status.json (1)

51-51: LGTM! Field rename aligns with the API change.

The field name change from vehicle_eta to time_to_full_charge correctly reflects the updated property implementation, which now reads this field and computes an absolute datetime.

openevsehttp/__main__.py (2)

6-6: LGTM! Import change improves specificity.

The updated import statement is more specific and aligns well with the usage in the file.

1147-1147: LGTM! Type annotation is consistent with the import change.

The simplified type annotation datetime | None is cleaner and consistent with the updated import statement.

tests/test_main.py (2)

11-12: LGTM! Necessary imports for updated test.

The datetime components and freezegun imports support the new test implementation that verifies datetime calculations.

1270-1290: LGTM! Test implementation correctly validates the datetime behavior.

The test updates properly:

Use time freezing for deterministic datetime assertions

Compute the expected datetime based on frozen time plus the expected seconds

Handle both scenarios (value present and None)

openevsehttp/__main__.py

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @openevsehttp/__main__.py:
- Around line 1146-1149: The OpenEVSE.time property is annotated as returning
datetime | None but actually returns the raw string from
self._status.get("time", None); update the property to annotate return type as
str | None and adjust the docstring to "Get the RTC time as ISO-8601 string."
(alternatively, if you prefer returning a datetime, change the implementation in
OpenEVSE.time to parse the ISO-8601 string from self._status.get("time") into a
datetime with proper UTC/Z handling and return that, ensuring None is
preserved).

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 239fcc7 and a0c61ad.

📒 Files selected for processing (1)

openevsehttp/__main__.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: coverage

🔇 Additional comments (1)

openevsehttp/__main__.py (1)

6-6: Import change is fine (timezone-aware UTC math).
Using timezone.utc with datetime.now() is the right direction for an absolute ETA.

openevsehttp/__main__.py

fix: change vehicle_eta to return datetime object or None

8e3a709

firstof9 marked this pull request as ready for review January 9, 2026 15:57

formatting

239fcc7

coderabbitai bot reviewed Jan 9, 2026

View reviewed changes

openevsehttp/__main__.py Show resolved Hide resolved

handle if time is 0

a0c61ad

coderabbitai bot reviewed Jan 9, 2026

View reviewed changes

openevsehttp/__main__.py Outdated Show resolved Hide resolved

openevsehttp/__main__.py Show resolved Hide resolved

firstof9 added 2 commits January 9, 2026 09:57

update time to return datetime object too

053b75c

update tests

9c8880f

firstof9 merged commit ffb0587 into main Jan 9, 2026
3 checks passed

firstof9 deleted the update-vehicle-eta branch January 9, 2026 17:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: change `vehicle_eta` to return datetime object or None #501

fix: change `vehicle_eta` to return datetime object or None #501

Uh oh!

firstof9 commented Jan 9, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 9, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: change vehicle_eta to return datetime object or None #501

fix: change vehicle_eta to return datetime object or None #501

Uh oh!

Conversation

firstof9 commented Jan 9, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: change `vehicle_eta` to return datetime object or None #501

fix: change `vehicle_eta` to return datetime object or None #501

firstof9 commented Jan 9, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 9, 2026 •

edited

Loading