fix(config): clean up auto-import for Shelly Wave 1PM #6289
Conversation
QubinoHelp
commented
Sep 18, 2023
QubinoHelp
commented
- added name manufacturer
- corrected Label & description
- fixed product Type & productId
- removed zwaveAllianceId (do not have one yet)
- added parameters description and options
- changed filename
|
@AlCalzone Can you please review these errors? Some of the test files report errors although we did not use them. They go unused. |
|
Will take a look tomorrow and let you know what needs to be changed. |
- updated label - updated description (product name) - corrected the productType & productId - added parameter descriptions
- added name manufacturer - corrected Label & description - removed zwaveAllianceId (do not have one yet) - added parameters description and options
- added read only state to parameter 201, 202, 203
- rename the file
|
Looks like you based your branch on the wrong branch from our repo - or maybe the merge messed it up. I fixed that, but I had to force-push to your branch. Before doing any further changes, please reset your local copy of the branch to the one on Github. Assuming you did not rename the remote, and you are on the correct branch locally, this command should do it: The file seems to be syntactically correct, I'll give you a more detailed style review now. |
There was a problem hiding this comment.
Thanks a lot! I have a few remarks, mostly related to consistency and following our style guide.
Feel free to ask if there is anything unclear or you need help.
| "paramInformation": [ | ||
| { | ||
| "#": "1", | ||
| "label": "Sw1 Switch Type", | ||
| "description": "0", | ||
| "description": "This parameter defines how the Device should treat the switch (which type) connected to the SW (SW1) terminal.", |
There was a problem hiding this comment.
We try to avoid parameter descriptions unless necessary for understanding what the parameter does, or to provide additional information like gaps in the allowed parameter range.
https://zwave-js.github.io/node-zwave-js/#/config-files/style-guide?id=parameter-descriptions
| }, | ||
| { | ||
| "#": "201", | ||
| "label": "Serial Number 1", | ||
| "description": "0", | ||
| "description": "This parameter contains a part of device’s serial number.\nThe parameter is Read-Only and cannot be changed.\nThe parameter is Advanced and may be hidden under the Advanced tag.", |
There was a problem hiding this comment.
I'd remove the description. That it is read-only is already included in the definition. And the last sentence doesn't give any useful information to the end user.
|
Thank you very much for the detailed report of the configuration file. I will go trough all of the suggested changes. What I did is practically took the descriptions from our database for the manuals. I will see what we can change in the actual database for the manuals to make them more compatible, most probably not everything can be done as suggested in this case I will have to add a new column in the database for the configuration files descriptions. |
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
|
Whoops, commented on the wrong PR |
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
Co-authored-by: AlCalzone <[email protected]>
AlCalzone
changed the title
|
I just did the remaining changes. |
AlCalzone
merged commit d827296
into
zwave-js:import-config-1694608670
