Skip to content
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 potential div zero error in init #95

Merged
merged 2 commits into from
Jan 9, 2025
Merged

Conversation

crkochan
Copy link
Contributor

@crkochan crkochan commented Jan 9, 2025

Wraps math function inside ternary operator to provide for a "default" value when actual values are not available.

I suspect this happens when the virtual meter device is created during local night, when the inverters are in an error state, and associated entites are unavailable.

Wraps math function inside ternary operator to provide for a "default" value when actual values are not available.
@crkochan crkochan mentioned this pull request Jan 9, 2025
Copy link
Owner

@krbaker krbaker left a comment

Choose a reason for hiding this comment

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

This shouldn't assume values if there are none, I think this should be a missing data point and I think None will plumb through here?

custom_components/sunpower/__init__.py Outdated Show resolved Hide resolved
@crkochan
Copy link
Contributor Author

crkochan commented Jan 9, 2025

Updated with feedback.

Also, let me know if you have a preference for full fat if/else instead of ternaries as the primary maintainer.

I have a tendency toward one-liners when making quick hacks, but for they can sometimes be not so great for code readability.

@krbaker
Copy link
Owner

krbaker commented Jan 9, 2025

Updated with feedback.

Also, let me know if you have a preference for full fat if/else instead of ternaries as the primary maintainer.

I have a tendency toward one-liners when making quick hacks, but for they can sometimes be not so great for code readability.

Just making sure this is tested and does what I'd expect (I'm not 100% sure, but believe this should result in unknown data for a sample with 0 inverters)

I'm perfectly happy with the ternaries, they are small enough they stay perfectly readable (IMO)

Copy link
Owner

@krbaker krbaker left a comment

Choose a reason for hiding this comment

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

so long as this does what I think go for it.

@crkochan
Copy link
Contributor Author

crkochan commented Jan 9, 2025

I'm testing just by editing the filed directly on my install through the VSCode add-on.

I have confirmed your surmise that the value being None make the resulting entitites Unavailable in Home Assistant.

In __init__.py, I set...

    # freq_avg = sum(freq) / len(freq) if len(freq) > 0 else None
    # volts_avg = sum(volts) / len(volts) if len(volts) > 0 else None
    freq_avg = None
    volts_avg = None

Result...
image

@krbaker krbaker merged commit 65a66e1 into krbaker:main Jan 9, 2025
1 check passed
@crkochan crkochan deleted the patch-1 branch January 9, 2025 19:02
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