Skip to content
This repository has been archived by the owner on Feb 11, 2021. It is now read-only.

Handle missing serial numbers #44

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

alexford
Copy link

After the discussion in #41 I figured I'd push up what I did to work around nodes without serial numbers available. I factored out that serial number retrieval into its own function.

Separately, I added a Jest test to help cover that case.

I realize choosing and adding Jest is a rather large suggestion/change. I'm happy to revert that if you'd rather not take that step, or use a different framework, etc. It was just a lot quicker to make sure this behaves as I expected by writing tests versus poking at real Z-Wave devices.

@mike182uk
Copy link
Owner

Hey @alexford

Sorry for the late reply, i completely missed this 🙈

Would you mind fixing the lint error and resolving the merge conflicts? Its probably easiest to pull in the package.json changes from master and running npm i to regenerate the package-lock.json rather than trying to resolve its conflicts manually 😃

I realize choosing and adding Jest is a rather large suggestion/change. I'm happy to revert that if you'd rather not take that step, or use a different framework, etc. It was just a lot quicker to make sure this behaves as I expected by writing tests versus poking at real Z-Wave devices.

Thanks for the taking the time to add this, its definitely something I should of got round to sooner rather than later. More than happy to keep it in 👍

@alexford alexford force-pushed the feature/optional-serial branch from ed718d5 to 0d92547 Compare September 13, 2020 09:58
@alexford
Copy link
Author

I've rebased and fixed the lint error. Tried to get the tests to run on CI but openzwave doesn't seem fully happy on Travis (https://travis-ci.org/github/mike182uk/homebridge-zwave/builds/726733400?utm_source=github_status&utm_medium=notification)

@mike182uk
Copy link
Owner

Ah maybe we need to add a jest config file and tell it to only run tests in the lib dir?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants