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

feat(discovery): add support for availability topics #3510

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Jan 8, 2024

Require the client to be connected (LWT will set to false when MQTT connection is lost because the process goes down), the driver to be ready, and the individual node to be online

require the client to be connected (LWT will set to false when
MQTT connection is lost because the process goes down), the driver
to be ready, and the individual node to be online
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! Did you tested it? Is it working well? I cannot test with HA or Domoticz as I don't use them.

You may need to wrap this in a method on the client and add it also to the discoverDevice method

@ccutrer
Copy link
Contributor Author

ccutrer commented Jan 9, 2024

I've tested it with the current releases of Home Assistant and openHAB (I primarily use the latter, and am the maintainer of its MQTT/Home Assistant compatibility layer). It's been working quite well - immediately showing the devices as offline if I kill zwave-js-ui, and then showing them as online when I restart it.

@ccutrer
Copy link
Contributor Author

ccutrer commented Jan 9, 2024

@zwave-js-bot fix lint

@coveralls
Copy link

coveralls commented Jan 9, 2024

Pull Request Test Coverage Report for Build 7464892756

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 22.174%

Totals Coverage Status
Change from base Build 7444718196: -0.03%
Covered Lines: 3737
Relevant Lines: 17925

💛 - Coveralls

so it can also be configured on custom devices
@ccutrer
Copy link
Contributor Author

ccutrer commented Jan 9, 2024

You may need to wrap this in a method on the client and add it also to the discoverDevice method

I've wrapped this in a method and tested the regular path, but I don't (currently) have any custom devices to test it with.

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

All good for the rest!

api/lib/Gateway.ts Outdated Show resolved Hide resolved
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM

@robertsLando robertsLando changed the title feat: configure availability topics for MQTT discovery feat(discovery): add support for availability topics Jan 10, 2024
@robertsLando robertsLando enabled auto-merge (squash) January 10, 2024 07:52
@robertsLando robertsLando merged commit e7ce406 into zwave-js:master Jan 10, 2024
9 checks passed
@ccutrer ccutrer deleted the hass-mqtt-availability branch January 10, 2024 13:59
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.

4 participants