Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Sonos: adjust log levels #4864

Merged
merged 1 commit into from Jan 9, 2018
Merged

Sonos: adjust log levels #4864

merged 1 commit into from Jan 9, 2018

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Jan 6, 2018

Fixes #4850

Signed-off-by: Laurent Garnier lg.hc@free.fr

@maggu2810
Copy link
Contributor

the problem is more that the binding tries to update all group members, for example the currently track information. As you have not these things, there is a log message.
But in this case (group slaves), I agree that we should just ignore them if the thing does not exist.
I will think about a fix.

If I understand this change correctly, you changed the log level if an illegal state exception has been catched. Is the reason "no such thing available" the only cause of that exception?

@kaikreuzer
Copy link
Contributor

Using info logging at these places is not in line with our logging guidelines. So I would say that it should either stay a warning (in case this is a software or general setup bug and we should usually never get here) or it should be logged as debug only.

@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 8, 2018

Yes, "No such thing available" is the only cause of the exception.

And if INFO level should never be used except for important things, yes we should replace with DEBUG level.

Fixes #4850

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 8, 2018

Review comments integrated.

@kaikreuzer kaikreuzer merged commit 0ef32d7 into eclipse-archived:master Jan 9, 2018
@lolodomo lolodomo deleted the sonos_warn branch January 9, 2018 12:40
dgajic pushed a commit to dgajic/smarthome that referenced this pull request Jan 27, 2018
Fixes eclipse-archived#4850

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@htreu htreu added this to the 0.10.0 milestone Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants