Session: store vehicle SoC at start and end#30050
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Using
if soc := lp.vehicleSoc; soc > 0to decide whether to persist SoC will silently drop valid 0% readings; consider using an explicit sentinel/flag for "unknown" SoC or checking>= 0if 0 is a valid value in your domain. - The new
SocStart/SocEndfields are*float64but marked withformat:"int"; if the values are truly integer percentages you might prefer an integer type, otherwise align theformattag with the actual type to avoid confusion for API consumers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `if soc := lp.vehicleSoc; soc > 0` to decide whether to persist SoC will silently drop valid 0% readings; consider using an explicit sentinel/flag for "unknown" SoC or checking `>= 0` if 0 is a valid value in your domain.
- The new `SocStart`/`SocEnd` fields are `*float64` but marked with `format:"int"`; if the values are truly integer percentages you might prefer an integer type, otherwise align the `format` tag with the actual type to avoid confusion for API consumers.
## Individual Comments
### Comment 1
<location path="core/session/session.go" line_range="25-26" />
<code_context>
MeterStop *float64 `json:"meterStop" csv:"Meter Stop (kWh)" gorm:"column:meter_end_kwh"`
ChargedEnergy float64 `json:"chargedEnergy" csv:"Charged Energy (kWh)" gorm:"column:charged_kwh"`
ChargeDuration *time.Duration `json:"chargeDuration" csv:"Charge Duration" gorm:"column:charge_duration"`
+ SocStart *float64 `json:"socStart" csv:"SoC Start (%)" gorm:"column:soc_start" format:"int"`
+ SocEnd *float64 `json:"socEnd" csv:"SoC End (%)" gorm:"column:soc_end" format:"int"`
SolarPercentage *float64 `json:"solarPercentage" csv:"Solar (%)" gorm:"column:solar_percentage"`
Price *float64 `json:"price" csv:"Price" gorm:"column:price"`
</code_context>
<issue_to_address>
**issue (bug_risk):** The `format:"int"` tag looks inconsistent with the `float64` type.
Both `SocStart` and `SocEnd` are `*float64` but use `format:"int"` in their tags. This mismatch can confuse consumers that rely on the tag for integer semantics (e.g. CSV export, API clients, UI formatting). Please either switch to an integer type if SoC is meant to be integral, or update/remove the `format:"int"` tag so it matches the actual type.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| SocStart *float64 `json:"socStart" csv:"SoC Start (%)" gorm:"column:soc_start" format:"int"` | ||
| SocEnd *float64 `json:"socEnd" csv:"SoC End (%)" gorm:"column:soc_end" format:"int"` |
There was a problem hiding this comment.
issue (bug_risk): The format:"int" tag looks inconsistent with the float64 type.
Both SocStart and SocEnd are *float64 but use format:"int" in their tags. This mismatch can confuse consumers that rely on the tag for integer semantics (e.g. CSV export, API clients, UI formatting). Please either switch to an integer type if SoC is meant to be integral, or update/remove the format:"int" tag so it matches the actual type.
| session.Created = lp.clock.Now() | ||
| if soc := lp.vehicleSoc; soc > 0 { | ||
| session.SocStart = &soc | ||
| } |
There was a problem hiding this comment.
Vehicle soc might not be present on session start.
I'd suggest moving this outside the if session.Created.IsZero() { check and add a session.SocStart == nil check.
|
Lets reuse the visualization from #19413 |
|
We need to exclude heaters since session entries dont have any information we can use to infer soc vs. temp formatting from. With the new metrics, heating devices should be excluded from sessions anyway soon. |
|
@copilot resolve the merge conflicts in this pull request |
# Conflicts: # i18n/de.json # i18n/en.json
Resolved and merged |
Skip SoC capture for heating devices since their reported value is a temperature, not a state of charge. Show the charged SoC delta and range in the session table and details, reusing the visualization from #19413.
Vehicle SoC may not be present when the session is created. Capture it on any charge-start segment while still unset instead of only at creation.
Records the vehicle's reported state of charge when a charging session
starts and when it ends. New fields are written to the
sessionstableand exposed in both the REST API (
/api/sessions) and the CSV export.socStart— vehicle SoC at the time charging begins (when thesession's
Createdtimestamp is first set)socEnd— vehicle SoC captured each time session energy metrics arepersisted (final value on
stopSession)Both fields are optional (
*float64) and only stored when a vehicle SoCis available, keeping the existing behaviour for off-line vehicles
unchanged.
Closes #30049
Replaces #19413