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

Improvements #83

Merged
merged 7 commits into from
Sep 24, 2019
Merged

Improvements #83

merged 7 commits into from
Sep 24, 2019

Conversation

cburghardt
Copy link
Contributor

  • central scene command class for channel naming
  • network layout improvements: better overview and colors based on hops from controller
  • preinstall: add -u option to unzip for Mac and add missing ldconfig call
    Version increase

@AlCalzone
Copy link
Collaborator

@@ -217,7 +217,8 @@ var comclasses = {
0x2F: {name: 'ZIP_ADV_SERVICES', role: ''},
0x2e: {name: 'ZIP_CLIENT', role: ''},
0x24: {name: 'ZIP_SERVER', role: ''},
0x23: {name: 'ZIP_SERVICES', role: ''}
0x23: {name: 'ZIP_SERVICES', role: ''},
0x5B: {name: 'CENTRAL_SCENE', role: ''}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to double check if there is a definition for all command classes, see
https://github.com/AlCalzone/node-zwave-js/blob/master/src/lib/commandclass/CommandClasses.ts#L4 for reference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, remaining com classes added.

@cburghardt
Copy link
Contributor Author

This change requires ldconfig to be allowed in
https://github.com/ioBroker/ioBroker/blob/master/installer.sh and
https://github.com/ioBroker/ioBroker/blob/master/fix_installation.sh

I fear this will create a lot of issues when the people don't know that the fixer is a prerequisite.
On the other side this missing ldconfig call is an issue as the installation doesn't work without. Isn't there a chance to prompt for password?

@AlCalzone
Copy link
Collaborator

Isn't there a chance to prompt for password?

No. I wouldn't know a foolsafe way to forward such a prompt to the admin UI.

The better approach would be ioBroker/ioBroker.js-controller#348 IMO - which means that each adapter specifies what root commands it needs. The js-controller would have to allow those (manage permissions) and then sudo ldconfig would be no issue.

@cburghardt
Copy link
Contributor Author

Reverted ldconfig change for now

@cburghardt
Copy link
Contributor Author

Ready from my side, the travis errors are just the usual strange host not found issues

@cburghardt
Copy link
Contributor Author

@AlCalzone or @Apollon77 can you merge if you are fine with that change please?

Copy link
Collaborator

@AlCalzone AlCalzone left a comment

Choose a reason for hiding this comment

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

LGTM

@AlCalzone AlCalzone merged commit 9a9f32c into ioBroker:master Sep 24, 2019
@AlCalzone
Copy link
Collaborator

I squash-merged, so please make sure to use a new branch for your next changes or reset it to the current master

@cburghardt cburghardt deleted the improvements branch September 24, 2019 08:24
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