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(ui, zwaveclient): learn/secondary controller mode #4097

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

Conversation

ArtemKiyashko
Copy link

@ArtemKiyashko ArtemKiyashko commented Jan 19, 2025

Add support of "learn mode". Controller can be added to pre-existing network and used as "secondary" controller.

Driver function ref

From Aeotec user manual:

Adding Z-Stick to a Pre-existing Z-Wave network.

This must be done through the host software which takes control of Z-Stick USB adapter while Z-Stick is in SerialAPI-Mode.
Please consult the instruction manual of the host software to add the Z-Stick to a pre- existing Z-Wave network (i.e. “Learn”, “Sync”, “Add as Secondary Controller”, etc.). This function can only be performed via host software.

Screenshots:

Screenshot 2025-01-19 at 13 54 40
image

@ArtemKiyashko ArtemKiyashko changed the title Add learn mode support feat:learn/secondary controller mode Jan 19, 2025
@ArtemKiyashko ArtemKiyashko changed the title feat:learn/secondary controller mode feat(ui):learn/secondary controller mode Jan 19, 2025
@ArtemKiyashko ArtemKiyashko changed the title feat(ui):learn/secondary controller mode feat(ui, zwaveclient): learn/secondary controller mode Jan 19, 2025
@robertsLando
Copy link
Member

@ArtemKiyashko Thanks for your PR, I was waiting before implementing this as @AlCalzone told me this is not ready for production yet. But I think we could maybe make this clear in the confirm text.

@AlCalzone WDYT?

Comment on lines +314 to +321
{
name: 'Start',
action: 'startLearnMode',
args: {
confirm:
'Initiate learn mode on primary controller first and then click OK here.',
},
},
Copy link
Member

Choose a reason for hiding this comment

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

I would also add the stop action here

Copy link
Author

Choose a reason for hiding this comment

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

Added
image

@robertsLando
Copy link
Member

Also it may have more sense to add this to the nodes manager dialog IMO

@ArtemKiyashko
Copy link
Author

Also it may have more sense to add this to the nodes manager dialog IMO

@robertsLando

Not sure if this is node specific functionality, because this is controller level function. E.g. no sense to show this option in Node dialog for controlled devices.

But i can move this option to node dialog if you wish

@robertsLando
Copy link
Member

Not sure if this is node specific functionality, because this is controller level function. E.g. no sense to show this option in Node dialog for controlled devices.

Let's see what @AlCalzone opinion about this :) For me it could be ok also as it is now

robertsLando
robertsLando previously approved these changes Jan 20, 2025
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.

Looks good from code side 👍🏼

@ArtemKiyashko
Copy link
Author

@robertsLando
Copy link
Member

@zwave-js-bot fix lint

@robertsLando
Copy link
Member

@ArtemKiyashko Lint issues, you should run npm run lint-fix but I asked bot to do it :)

@robertsLando
Copy link
Member

Ok seems bot is having some issues: https://github.com/zwave-js/zwave-js-ui/actions/runs/12869249637/job/35877613209

@ArtemKiyashko could you do that please? Just run that command and it will fix lint

@ArtemKiyashko
Copy link
Author

Ok, seems it was "newline" issue. Should be good now

Ok seems bot is having some issues: https://github.com/zwave-js/zwave-js-ui/actions/runs/12869249637/job/35877613209

@ArtemKiyashko could you do that please? Just run that command and it will fix lint

@coveralls
Copy link

Pull Request Test Coverage Report for Build 12869310829

Details

  • 0 of 49 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 21.008%

Changes Missing Coverage Covered Lines Changed/Added Lines %
api/lib/ZwaveClient.ts 0 49 0.0%
Totals Coverage Status
Change from base Build 12866617879: -0.05%
Covered Lines: 3952
Relevant Lines: 20010

💛 - Coveralls

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.

3 participants