Skip to content

Mqtt: reset full/empty times#29050

Open
andig wants to merge 2 commits intomasterfrom
fix/full-empty
Open

Mqtt: reset full/empty times#29050
andig wants to merge 2 commits intomasterfrom
fix/full-empty

Conversation

@andig
Copy link
Copy Markdown
Member

@andig andig commented Apr 12, 2026

Fix #28811

@andig andig added the enhancement New feature or request label Apr 12, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

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

  • Changing Full/Empty from *time.Time to time.Time removes the explicit nil/no-value state; double-check all callers and MQTT consumers correctly interpret the zero time.Time and that this doesn’t introduce ambiguity with legitimate time.Time{} values.
  • The struct tags json:"full,omitzero" / json:"empty,omitzero" are non-standard for Go’s encoding/json (which only understands omitempty); confirm that your JSON marshaller actually supports omitzero or adjust the tags to match the serializer being used.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Changing `Full`/`Empty` from `*time.Time` to `time.Time` removes the explicit nil/no-value state; double-check all callers and MQTT consumers correctly interpret the zero `time.Time` and that this doesn’t introduce ambiguity with legitimate `time.Time{}` values.
- The struct tags `json:"full,omitzero"` / `json:"empty,omitzero"` are non-standard for Go’s `encoding/json` (which only understands `omitempty`); confirm that your JSON marshaller actually supports `omitzero` or adjust the tags to match the serializer being used.

## Individual Comments

### Comment 1
<location path="core/site_optimizer.go" line_range="73-74" />
<code_context>
 	batteryDetail
-	Full  *time.Time `json:"full"`
-	Empty *time.Time `json:"empty"`
+	Full  time.Time `json:"full,omitzero"`
+	Empty time.Time `json:"empty,omitzero"`
 }

</code_context>
<issue_to_address>
**issue (bug_risk):** Using `omitzero` in JSON tags is non-standard and likely ignored by `encoding/json`

`encoding/json` only supports the `omitempty` option. With `json:"full,omitzero"`, `Full` will still always be marshaled, and a zero `time.Time` becomes `"0001-01-01T00:00:00Z"`. If you need to omit zero timestamps, you’ll need a custom `MarshalJSON` (on `batteryResult` or a wrapper `time.Time` type) rather than relying on this tag option.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +73 to +74
Full time.Time `json:"full,omitzero"`
Empty time.Time `json:"empty,omitzero"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Using omitzero in JSON tags is non-standard and likely ignored by encoding/json

encoding/json only supports the omitempty option. With json:"full,omitzero", Full will still always be marshaled, and a zero time.Time becomes "0001-01-01T00:00:00Z". If you need to omit zero timestamps, you’ll need a custom MarshalJSON (on batteryResult or a wrapper time.Time type) rather than relying on this tag option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MQTT battery forecast not cleared from broker

1 participant