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

availability_whitelist #2387

Merged
merged 4 commits into from
Nov 23, 2019
Merged

availability_whitelist #2387

merged 4 commits into from
Nov 23, 2019

Conversation

Kryzek
Copy link

@Kryzek Kryzek commented Nov 22, 2019

As requested at #2204

Tested with my environment

With

  availability_timeout: 120
  availability_whitelist: ['0x00124b001d3ab1b9','0x001788010278b40a']

in configuration.yaml

I get this behaviour as wanted:

zigbee2mqtt:debug 2019-11-22 14:08:09: Received Zigbee message from '0x00124b001d3ab1b9', type 'readResponse', cluster 'genBasic', data '{"zclVersion":1}' from endpoint 11 with groupID 0
zigbee2mqtt:debug 2019-11-22 14:08:09: Successfully pinged '0x00124b001d3ab1b9'
...
zigbee2mqtt:info  2019-11-22 14:08:19: MQTT publish: topic 'zigbee/0x001788010278b40a/availability', payload 'offline'
zigbee2mqtt:error 2019-11-22 14:08:19: Failed to ping '0x001788010278b40a'
...
zigbee2mqtt:debug 2019-11-22 14:18:11: Successfully pinged '0x00124b001d3ab1b9'
zigbee2mqtt:debug 2019-11-22 14:18:49: Failed to ping '0x001788010278b40a'

Hope I did not break anything else while at it.

Closes #2204

@Kryzek Kryzek mentioned this pull request Nov 22, 2019
7 tasks
@Koenkk
Copy link
Owner

Koenkk commented Nov 22, 2019

This feature looks good to me, if you can add the missing tests than this can be merged, see https://github.com/Koenkk/zigbee2mqtt/blob/master/test/deviceAvailability.test.js#L207 for inspiration.

@Kryzek
Copy link
Author

Kryzek commented Nov 22, 2019

If my Google-Fu is not terribly bad, the goal is to get npm test -- --coverage to run without errors?

@Koenkk
Copy link
Owner

Koenkk commented Nov 22, 2019

Yes: npm run test-with-coverage. The coverage report (index.html) can be found after this in the coverage folder.

@Kryzek
Copy link
Author

Kryzek commented Nov 22, 2019

I'll be damned! Tests passed! 💪

@Kryzek
Copy link
Author

Kryzek commented Nov 22, 2019

Also updated documentation Koenkk/zigbee2mqtt.io#165

@Koenkk
Copy link
Owner

Koenkk commented Nov 22, 2019

I reshuffled the logic a bit, tests are still passing, is this OK for you?

@Kryzek
Copy link
Author

Kryzek commented Nov 23, 2019

Wow... I really overthought this. ’return this.whitelist.includes(device.ieeeAddr);’ does return also false... So much cleaner now.

How about those forcePinged devices, should those be checked before?

@Koenkk
Copy link
Owner

Koenkk commented Nov 23, 2019

Forced pinged devices is a list of end devices which can be pinged, however the whitelist does basically the same. It would be strange if a device is NOT in the whitelist but is still pinged.

@Kryzek
Copy link
Author

Kryzek commented Nov 23, 2019

Makes sense.

I’m happy with your changes since they make same as mine but just much more elegant way.

@Koenkk
Copy link
Owner

Koenkk commented Nov 23, 2019

Thanks!

@Koenkk Koenkk merged commit 46d7176 into Koenkk:dev Nov 23, 2019
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.

2 participants