Skip to content

Conversation

feliciaan
Copy link
Contributor

Skip attributes and add warning for missing firmware info in charger attributes, prevents the integration from crashing while starting, if these attributes are not available.

This only happens for chargers that are added to the Zaptec portal, but not configured or correctly added.

Add warning for missing firmware info in charger attributes, prevents the integration from crashing while starting.

This only happens for chargers that are added to the Zaptec portal, but not configured or correctly added.
charger = self.zaptec.get(fm["ChargerId"])
if charger is None:
continue
if fm.get("CurrentVersion") is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe do the check for all 3 keys just to be sure there's no obscure case where one key is defined while the others aren't?

@sveinse
Copy link
Collaborator

sveinse commented Sep 28, 2025

@feliciaan Is this issue critical enough to delay the release of 0.8.3 ? Should we wait for this one?

@sveinse sveinse mentioned this pull request Sep 28, 2025
@feliciaan
Copy link
Contributor Author

I think it's complete, so it can be merged, but I also think it's not a high priority issue, so it can be put in the next release.

):
# If the charger is already added to the Zaptec platform but not yet
# initialized, these fields are not available.
_LOGGER.warning("Missing firmware info for charger %s", charger.qual_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a warning that will be pushed to be visible to the users. Are we certain the error message is explanatory enough without the context? Can we foresee a issue report from users on this message? -- Perhaps something like "Missing firmware info from charger %s because the charger haven't been initialized yet. Safe to ignore" or something alike.

Copy link
Contributor

Choose a reason for hiding this comment

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

@feliciaan Please respond, so that we can include this in the upcoming 0.8.4 release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it, I think it's ready to merge.

@sveinse sveinse added this to the v0.8.4 milestone Sep 29, 2025
Copy link
Collaborator

@sveinse sveinse left a comment

Choose a reason for hiding this comment

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

Looks good now

@sveinse sveinse merged commit 11076ea into custom-components:master Oct 12, 2025
6 checks passed
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.

3 participants