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

Update FCM debug log to match Wear OS for better debugging data #4391

Closed

Conversation

dshokouhi
Copy link
Member

Summary

Noticed our debug log for phone FCM messages did not contain all the important debugging data like notification content. So matching Wear OS output here for easier debugging. Websocket messages is already covered.

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

@jpelgrom
Copy link
Member

I'm not really a big fan of logging so much, even if we do it on Wear. The received notification is immediately stored:

notificationId = notificationDao.add(notificationRow)

and unlike Wear, it can be viewed from the app settings (as a JSON object so almost raw!). What are you trying to cover with the log statement that isn't available here? Maybe it's better to expose that in settings.

Websocket messages is already covered.

Unless I'm missing something, WS doesn't do this. Message contents are only logged on debug builds, and WebsocketManager doesn't log contents for received messages either.

(This is why I'm late to comment on this PR - wanted to take the time to consider and see what we already are doing.)

@dshokouhi
Copy link
Member Author

was helping someone troubleshoot an issue and wanted to check the output it was receiving, agreed we dont need this as the history on the phone will be good enough.

@dshokouhi dshokouhi closed this May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants