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

Change log level of some logs to debug #274

Conversation

mvdwetering
Copy link
Contributor

I had to analyze my HA logs for another issue and it was difficult due to the Omnik integration generating a lot of logging during the night.

I would suggest to change these logs level to debug to avoid polluting the logs when not needed. When needed for debugging one can easily enable debug logging for the integration to get them again.

I had this running for a couple of days and seems to work as intended.

@MarijnS95
Copy link
Collaborator

MarijnS95 commented Dec 30, 2023

Some of these should not just serve as application debug, but as useful info when e.g. setting up the connection and getting a failed connection without details otherwise. It is hard to tell from the diff if this is now affected, but it looks like it.

@mvdwetering
Copy link
Contributor Author

mvdwetering commented Dec 31, 2023

The errorhandling during initial connection setup is not impacted as that is all in config_flow.py.

Config entries will still be shown in the HA UI as having an error. It is just not spamming the log anymore.
The ConnectionError is most of the time not an error for this integration as it is expected to not being able to connect when there is no sun. Hence the desire to not have logging for things that are expected to happen.

When the more detailed info is needed you can just click the "Enable debug logging" in the integration and it will show again (including stack traces).
image

I could revert the AuthenticaitonError change as that is likely to be an actual error. I just included it because it seemed the same as the other line.

@robbinjanssen
Copy link
Owner

Maybe we should change this into a setting via config flow?

@mvdwetering
Copy link
Contributor Author

Maybe we should change this into a setting via config flow?

Wouldn't that be almost the same as the Home Assistant "Enable debug logging" option?

Main difference would be that you can enable/disable it per config entry. "Enable debug logging" is for the whole integration so all config entries. Not sure how important that is.

@MarijnS95
Copy link
Collaborator

The errorhandling during initial connection setup is not impacted as that is all in config_flow.py.

Good to have that confirmed, that's where such errors are most helpful.

Config entries will still be shown in the HA UI as having an error. It is just not spamming the log anymore.

Do you mean shown as error while going through the config flow? Or does it show an error every night while the inverter is unreachable?

@mvdwetering
Copy link
Contributor Author

Config entries will still be shown in the HA UI as having an error. It is just not spamming the log anymore.

Do you mean shown as error while going through the config flow? Or does it show an error every night while the inverter is unreachable?

I meant this part of the UI
image

However, looking back I am not sure why I mentioned it since it only gets in that state when the integration gets (re)loaded.

@klaasnicolaas
Copy link
Collaborator

That is because your inverter is currently off and therefore not available to collect data?

@robbinjanssen
Copy link
Owner

Since this is no longer needed, i'll close this PR.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants