-
-
Notifications
You must be signed in to change notification settings - Fork 1
show trains after midnight next day #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
With this fix the sensors will also show trains after midnight - otherwise the list empties out after 23:00.
WalkthroughEnhanced departure time parsing in _async_update_data to roll over to the next day when an HH:MM value is earlier than the current time. Existing error handling for invalid formats remains. Minor non-functional formatting tweak (trailing newline in async_unload_entry). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor HA as Home Assistant
participant Comp as db_infoscreen component
participant Time as datetime.now()
HA->>Comp: _async_update_data()
Comp->>Time: now = datetime.now()
loop For each entry
Comp->>Comp: Parse HH:MM
alt Invalid time format
Comp->>Comp: Log error, skip entry
else Valid time
Comp->>Comp: candidate = today @ HH:MM
alt candidate < now
Comp->>Comp: candidate = candidate + 1 day
end
Comp->>Comp: departure_time = candidate
end
end
Comp-->>HA: Updated data
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limits.
🔭 Outside diff range comments (3)
custom_components/db_infoscreen/__init__.py (3)
244-251
: Arrival parsing: add next-day rollover for HH:MM and support ISO with seconds (consistency with departure).Right now, arrivals parsed from HH:MM are pinned to “today” without rolling over to the next day. After 23:xx, this can mislabel midnight+ arrivals and diverge from the fixed departure handling.
Apply this diff to mirror the departure parsing behavior and accept both ISO forms:
- if "T" in scheduled_arrival: - arrival_time = datetime.strptime(scheduled_arrival, "%Y-%m-%dT%H:%M") - else: - now = datetime.now() - arrival_time = datetime.strptime(scheduled_arrival, "%H:%M").replace( - year=now.year, month=now.month, day=now.day - ) + try: + # Try ISO with seconds + arrival_time = datetime.strptime(scheduled_arrival, "%Y-%m-%dT%H:%M:%S") + except ValueError: + try: + # Try ISO without seconds + arrival_time = datetime.strptime(scheduled_arrival, "%Y-%m-%dT%H:%M") + except ValueError: + # Fallback to HH:MM with next-day rollover + now = datetime.now() + arrival_candidate = datetime.strptime(scheduled_arrival, "%H:%M").replace( + year=now.year, month=now.month, day=now.day + ) + if arrival_candidate < now: + arrival_candidate += timedelta(days=1) + arrival_time = arrival_candidateAlso applies to: 242-259
276-285
: Bug: Delay is added to departure_time twice.
departure_time
is incremented at Line 277 and again at Line 285, effectively double-counting the delay. This skews offset filtering and displayed times.Remove the first addition (keep the later block that logs and handles fallback):
- if not self.drop_late_trains: - departure_time += timedelta(minutes=delay_departure or 0) - # Apply any delay to the departure time, if applicable if not self.drop_late_trains: _LOGGER.debug("Departure time without added delay: %s", departure_time) if delay_departure is None: delay_departure = 0 # Set delay to 0 as fallback if no valid delay has been found departure_time += timedelta(minutes=delay_departure) _LOGGER.debug("Departure time with added delay: %s", departure_time)
352-352
: Possible type bug: default fordrop_late_trains
is a list, but used as a boolean.
drop_late_trains
is handled as a boolean (e.g.,if not self.drop_late_trains:
), but here the default is[]
. This leads to surprising behavior (empty list is falsy).Use a boolean default:
- drop_late_trains = config_entry.data.get(CONF_DROP_LATE_TRAINS, []) + drop_late_trains = config_entry.data.get(CONF_DROP_LATE_TRAINS, False)
🧹 Nitpick comments (2)
custom_components/db_infoscreen/__init__.py (2)
263-275
: Dead/unreachable branch and potential AttributeError on string.date()
delay_arrival
is set to 0 whenNone
(Lines 239–241). Therefore,elif delay_arrival is None:
(Line 263) never executes. If it did,scheduled_arrival
is a string here and calling.date()
would raise.
- Remove the unreachable
elif delay_arrival is None:
block entirely.- If you want to keep the “no delay” path explicit, parse
scheduled_arrival
to a datetime (using the same logic as above) before formatting.I can provide a follow-up patch removing that branch if you prefer.
118-132
: Optional: Use timezone-aware now() from Home Assistant and compute it once.Multiple calls to
datetime.now()
produce naive datetimes and may cross minute boundaries mid-loop. Home Assistant providesdt_util.now()
(timezone-aware). Computenow
once at the start and reuse.
- Add import:
from homeassistant.util import dt as dt_util
- At the start of
_async_update_data
, capturenow = dt_util.now()
, and reuse it in:
- setting
self.last_update
- departure and arrival parsing rollovers
- date comparisons for formatting
- offset calculation
If you want, I can prepare a targeted patch adjusting these call sites.
Also applies to: 195-205, 247-251, 220-224, 229-233, 255-258
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
custom_components/db_infoscreen/__init__.py
(2 hunks)
🔇 Additional comments (2)
custom_components/db_infoscreen/__init__.py (2)
195-205
: LGTM on the midnight rollover approach.The “if candidate < now then +1 day” logic is the right fix to show departures after midnight on the next day.
373-373
: LGTM: trailing newline preservation in async_unload_entry.No functional change; consistent formatting.
Thanks! |
With this fix the sensors will also show trains after midnight - otherwise the list empties out after 23:00.
Proposed Changes
My private departure monitor was not showing trains after midnight on the next day. So the list became smaller and smaller after 23:00 and was always completely empty shortly before midnight. Right after midnight the "missing" trains were then shown.
Related Issues
[Bug]: Trains after midnight are not shown (#50)
Summary by CodeRabbit