[Fiat] Fix edge case causing charge to never stop

[FraBoCH]

Fix edge case of charge not stopping if charge session runs since the night before

[sourcery-ai]

Hey - I've left some high level feedback:

• Consider calling time.Now() only once, storing it in a variable, and reusing it for StartTime, EndTime, and nowStr to avoid subtle inconsistencies when the time ticks between calls.
• The hardcoded "00:01" fallback for StartTime is a bit opaque; consider making this a named constant or explaining the rationale so the intent of this edge-case handling is clear to future maintainers.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider calling `time.Now()` only once, storing it in a variable, and reusing it for `StartTime`, `EndTime`, and `nowStr` to avoid subtle inconsistencies when the time ticks between calls.
- The hardcoded "00:01" fallback for `StartTime` is a bit opaque; consider making this a named constant or explaining the rationale so the intent of this edge-case handling is clear to future maintainers.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The new stop-charging logic parses StartTime using time.Parse, which assumes UTC, while now comes from time.Now() (local time); if the schedule semantics are local-time-based, consider time.ParseInLocation with the appropriate location for consistency.
• The fallbackStartTime constant ("00:01") is a bit opaque; consider either deriving it from a small helper (e.g. midnightPlus(time.Minute)) or adding a brief comment explaining why that specific time is chosen to handle the cross-midnight edge case.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new stop-charging logic parses `StartTime` using `time.Parse`, which assumes UTC, while `now` comes from `time.Now()` (local time); if the schedule semantics are local-time-based, consider `time.ParseInLocation` with the appropriate location for consistency.
- The `fallbackStartTime` constant (`"00:01"`) is a bit opaque; consider either deriving it from a small helper (e.g. `midnightPlus(time.Minute)`) or adding a brief comment explaining why that specific time is chosen to handle the cross-midnight edge case.

## Individual Comments

### Comment 1
<location> `vehicle/fiat/controller.go:78-79` </location>
<code_context>
+		}
+		stat.EvInfo.Schedules[0].EndTime = rounded.Format(timeFormat)
+
+		// Parse times for comparison and handle edge case: StartTime > EndTime
+		if start, err := time.Parse(timeFormat, stat.EvInfo.Schedules[0].StartTime); err == nil && start.After(rounded) {
+			stat.EvInfo.Schedules[0].StartTime = fallbackStartTime
+		}
</code_context>

<issue_to_address>
**issue (bug_risk):** Comparing a clock-only parsed time to `rounded` mixes date components and likely never triggers as intended.

Here `start` is parsed with layout `15:04`, so it has year 0 in UTC, whereas `rounded` has the current date in local time. Comparing them with `start.After(rounded)` means `start` will effectively always be before `rounded`, so this edge-case path likely never executes. If you intend a clock-time comparison, normalize both values first (e.g., re-parse `rounded.Format(timeFormat)` with the same layout, or compare `Hour`/`Minute` directly).
</issue_to_address>

### Comment 2
<location> `vehicle/fiat/controller.go:71` </location>
<code_context>
 	} else {
-		// stop charging by updating active charge schedule end time to now
-		stat.EvInfo.Schedules[0].EndTime = time.Now().Format("15:04") // only hour and minutes
+		// Stop charging: update end time, rounded to next 5 mninutes
+		rounded := now.Truncate(5 * time.Minute)
+		if rounded.Before(now) {
</code_context>

<issue_to_address>
**nitpick (typo):** Minor typo in the comment for the 5-minute rounding.

Spelling: "mninutes" → "minutes".

```suggestion
		// Stop charging: update end time, rounded to next 5 minutes
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[andig]

I repeat my comment. Use time.After- this comparison is bound to be wrong.

[FraBoCH]

I repeat my comment. Use time.After- this comparison is bound to be wrong.

Ok, please find another simple implementation without risking date, Time zones or comparison issues

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The parsing and formatting of times uses the default location (UTC) via time.Parse, which may not match the timezone assumed by the schedule API; consider using time.ParseInLocation with the same location as used for time.Now() or whatever the Fiat API expects.
• On time parsing errors for StartTime/EndTime you currently fail silently; consider at least logging when err1/err2 are non-nil so unexpected schedule formats don't quietly bypass the midnight edge-case handling.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The parsing and formatting of times uses the default location (UTC) via `time.Parse`, which may not match the timezone assumed by the schedule API; consider using `time.ParseInLocation` with the same location as used for `time.Now()` or whatever the Fiat API expects.
- On time parsing errors for `StartTime`/`EndTime` you currently fail silently; consider at least logging when `err1`/`err2` are non-nil so unexpected schedule formats don't quietly bypass the midnight edge-case handling.

## Individual Comments

### Comment 1
<location> `vehicle/fiat/controller.go:78-81` </location>
<code_context>
+		}
+		stat.EvInfo.Schedules[0].EndTime = roundedEndTime.Format(timeFormat)
+
+		// Make sure start time is always before end time (parse both from string to ensure proper comparison)
+		start, err1 := time.Parse(timeFormat, stat.EvInfo.Schedules[0].StartTime)
+		end, err2 := time.Parse(timeFormat, stat.EvInfo.Schedules[0].EndTime)
+		if err1 == nil && err2 == nil && start.After(end) {
+			stat.EvInfo.Schedules[0].StartTime = fallbackStartTime
+		}
</code_context>

<issue_to_address>
**issue:** Schedules that intentionally cross midnight may be unintentionally rewritten.

Because `start.After(end)` triggers a reset to `fallbackStartTime`, a valid cross-midnight window like `23:00`–`01:00` will be treated as invalid and rewritten to `00:01`. If cross-midnight schedules are allowed, this will silently alter user settings. You could instead treat `end < start` as a next-day boundary (a valid cross-midnight case) rather than forcing `start` to the fallback value.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue: Schedules that intentionally cross midnight may be unintentionally rewritten.

Because start.After(end) triggers a reset to fallbackStartTime, a valid cross-midnight window like 23:00–01:00 will be treated as invalid and rewritten to 00:01. If cross-midnight schedules are allowed, this will silently alter user settings. You could instead treat end < start as a next-day boundary (a valid cross-midnight case) rather than forcing start to the fallback value.

[andig]

Happy to merge though I don‘t understand what you‘re trying to achieve