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

Quieter Logging #40

Merged
merged 6 commits into from
Jan 16, 2023
Merged

Conversation

clayoster
Copy link

This pull request makes the following changes and is related to issue #37:

  • Move existing logging to debug mode as it is more verbose than what I'd expect to see in the normal Homebridge log. These logs can now be found by enabling Homebridge Debug Mode
  • Add logging of state changes that simply show when the state changes to "Occupied" or "Unoccupied"
  • Add an option to disable the logging of state changes

Overall, this really cleans up the output to the Homebridge logs while allowing useful state change logging to be captured if desired.

Copy link
Owner

@Jason-Morcos Jason-Morcos left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you for the contribution.

I have two little tweaks to suggest and then I can get this wrapped into the next release!

config.schema.json Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated
@@ -480,7 +481,16 @@ class MagicOccupancy {
}

_setOccupancyState (newVal) {
// Capture the previous state and determine if the state change should be logged
this.previousModeState = this.modeState;
this.occupancyLoggingCondition = (newVal == 'Occupied' && this.previousModeState == 'Unoccupied' || newVal == 'Unoccupied')
Copy link
Owner

Choose a reason for hiding this comment

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

Last little thing - this shouldn't be a class-wide variable. I would probably use const here instead

const occupancyLoggingCondition = (newVal == 'Occupied' && this.previousModeState == 'Unoccupied' || newVal == 'Unoccupied')
if (this.occupancyLogging && occupancyLoggingCondition) {

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense. Thanks for the suggestion! The change has been made in dc54e03.

Copy link
Owner

@Jason-Morcos Jason-Morcos left a comment

Choose a reason for hiding this comment

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

Great! Thank you for the contribution!

@Jason-Morcos Jason-Morcos merged commit bfe9742 into Jason-Morcos:master Jan 16, 2023
@clayoster clayoster deleted the quieter-logging branch January 16, 2023 19:59
@clayoster
Copy link
Author

Thanks for your help along the way!

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.

None yet

2 participants