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

[bug] Do not start ZJS Server before the driver ready event #602

Closed
3 tasks done
balloob opened this issue Feb 17, 2021 · 4 comments · Fixed by #605
Closed
3 tasks done

[bug] Do not start ZJS Server before the driver ready event #602

balloob opened this issue Feb 17, 2021 · 4 comments · Fixed by #605
Assignees
Labels
bug Something isn't working

Comments

@balloob
Copy link

balloob commented Feb 17, 2021

Version

Build/Run method

  • Docker
  • PKG
  • Manually built (git clone - npm install - npm run build )

zwavejs2mqtt version: 1.1.1
zwavejs version: 6.4.0

Describe the bug

Z-Wave JS 2 MQTT will start the Z-Wave JS Server before the Driver object has emitted the driver ready event. This is causing bad information to be sent by Z-Wave JS Server. This is not supported.

Related bugs:

home-assistant/core#46180 (comment)

home-assistant/core#46279

home-assistant-libs/zwave-js-server-python#102

Bugs are closed because we now handle it from a Python side correctly if we receive bad data. Better would be if we didn't had the error to begin with.

CC @MartinHjelmare

To Reproduce

Steps to reproduce the behavior:

  1. Set up Home Assistant to connect to ZJS2MQTT via the ZJS Server
  2. Restart ZJS2MQTT

Expected behavior

The WS server should not come up until the driver is ready to serve data.

@balloob balloob added the bug Something isn't working label Feb 17, 2021
@balloob
Copy link
Author

balloob commented Feb 17, 2021

An alternative could be that we update Server.start to not start the server until the driver is ready. However that seems weird because start then doesn't automatically start.

I have a draft PR to update Server.start() to raise if the driver is not ready. That would help make it clear we're not ready to serve yet. Won't merge that until this issue has been resolved.

@robertsLando
Copy link
Member

Let me fix this :)

@balloob
Copy link
Author

balloob commented Feb 17, 2021

Thanks!

@balloob
Copy link
Author

balloob commented Feb 17, 2021

@zwavejs/server 1.0.0-beta.7 will now raise if the driver is not ready when the server starts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants