-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: ignore cfos 0.0 #24997
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
fix: ignore cfos 0.0 #24997
Conversation
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `charger/cfos.go:170-171` </location>
<code_context>
-
- return float64(binary.BigEndian.Uint64(b)) / 1e3, nil
+ result := float64(binary.BigEndian.Uint64(b)) / 1e3
+ if result == 0.0 {
+ return 0, errors.New("cfos returned 0")
+ }
+ return result, nil
</code_context>
<issue_to_address>
**issue (bug_risk):** Returning an error for zero energy may mask legitimate zero readings.
Zero energy readings can occur legitimately, such as after initialization. Consider handling zero values without treating them as errors to prevent misclassification.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
charger/cfos.go
Outdated
| if result == 0.0 { | ||
| return 0, errors.New("cfos returned 0") |
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.
issue (bug_risk): Returning an error for zero energy may mask legitimate zero readings.
Zero energy readings can occur legitimately, such as after initialization. Consider handling zero values without treating them as errors to prevent misclassification.
|
A more robust approach might be to store previous read values in the CfosPowerBrain structure and return the stored value in case it reports 0. Only storing values that are not 0 then makes sure it never reports 0 except on completely new cfos chargers. |
caching now the previous value |
Co-authored-by: andig <[email protected]>
|
Änderungen übernommen. Danke. @andig ist ein Hinweis in https://github.com/evcc-io/docs/blob/main/docs/devices/chargers.mdx bzgl. der Instabilität der Wallbox bei 1p3p Umschaltungen die zu den regelmäßigen Neustarts gewünscht? |
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.
ignore invalid total kWh - cfos returns 0.0
see #12886 (reply in thread)