Skip to content

Conversation

@springfall2008
Copy link
Owner

@springfall2008 springfall2008 commented Nov 19, 2025

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes timezone handling issues in the forecast.solar integration by requesting UTC timestamps from the API and removing local-to-UTC timezone conversion logic.

Key Changes:

  • Added ?time=utc query parameter to forecast.solar API URLs to receive timestamps in UTC
  • Removed custom TIME_FORMAT_FORECAST_SOLAR constant and timezone offset calculations
  • Updated version from v8.27.25 to v8.27.26

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
apps/predbat/config.py Removed unused TIME_FORMAT_FORECAST_SOLAR constant
apps/predbat/solcast.py Updated API URLs with ?time=utc parameter, switched to TIME_FORMAT for parsing, and removed timezone offset conversion logic
apps/predbat/predbat.py Version bump to v8.27.26

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

forecast_watt_data = {}
for period_end in watts:
period_end_stamp = datetime.strptime(period_end, TIME_FORMAT_FORECAST_SOLAR)
period_end_stamp = datetime.strptime(period_end, TIME_FORMAT)
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential datetime parsing issue: TIME_FORMAT is defined as "%Y-%m-%dT%H:%M:%S%z" which expects timezone offset in +HHMM or +HH:MM format. However, when the forecast.solar API returns UTC timestamps (with ?time=utc parameter), they typically use either:

  1. ISO 8601 format with 'Z' suffix (e.g., "2024-01-15T12:00:00Z"), or
  2. Space-separated format without timezone (e.g., "2024-01-15 12:00:00")

Python's %z directive doesn't recognize the 'Z' literal. If the API returns format (1), you'd need to replace 'Z' with '+00:00' before parsing. If it returns format (2), the 'T' separator won't match.

Please verify the actual format returned by the API with ?time=utc and adjust the parsing accordingly.

Copilot uses AI. Check for mistakes.
continue

current_time = info.get("time", None)
current_time_stamp = datetime.strptime(current_time, TIME_FORMAT)
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same parsing concern as line 288: verify that the info.get("time") value from the forecast.solar API response matches the TIME_FORMAT pattern, especially regarding timezone representation.

Copilot uses AI. Check for mistakes.
@springfall2008 springfall2008 merged commit 28e1285 into main Nov 19, 2025
7 checks passed
@springfall2008 springfall2008 deleted the myfixes6 branch November 19, 2025 08:21
iangregory pushed a commit to iangregory/batpred that referenced this pull request Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants