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(config): add Namron 4512748 2A Awning Controller #7088

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Swoop86
Copy link

@Swoop86 Swoop86 commented Aug 5, 2024

First time in a long time doing this, so need some assistance/guidance to get it right

Solves issue #7087

First time in a long time doing this, so need some assistance/guidance to get it right
@zwave-js-assistant zwave-js-assistant bot added the config ⚙ Configuration issues or updates label Aug 5, 2024
Last time testing, if it breaks again I'll install the environment...
I know what I said, this is the last time...
Comment on lines +81 to +100
{
"label": "Value=0",
"value": 0
},
{
"label": "Value=1",
"value": 1
},
{
"label": "Value=2",
"value": 2
},
{
"label": "Value=3",
"value": 3
},
{
"label": "Value=4",
"value": 4
}
Copy link
Member

Choose a reason for hiding this comment

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

What do those values mean? "Value=3" isn't self-explanatory.

Comment on lines +137 to +139
"label": "Power Change Threshold Limit",
"valueSize": 1,
"defaultValue": 10
Copy link
Member

Choose a reason for hiding this comment

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

This and the following parameters are missing:

  • unit (can also be sth. like "0.1 V")
  • mininum and maximum value

},
{
"#": "14",
"label": "Time Cycle To Report Energy Consumption Value",
Copy link
Member

Choose a reason for hiding this comment

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

We typically reword the labels to something more understandable, in this case:

Suggested change
"label": "Time Cycle To Report Energy Consumption Value",
"label": "Energy Report Interval",

},
{
"#": "6",
"label": "Starting Positioning Calibration Performed",
Copy link
Member

Choose a reason for hiding this comment

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

This sounds like it indicates a status, but is writeonly?

"paramInformation": [
{
"#": "2",
"label": "Enable/Disable State Report Threshold",
Copy link
Member

Choose a reason for hiding this comment

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

Avoid using "Enable/Disable" in labels. Usually there is a better label, or the options already imply that they are meant for enabling or disabling something.

@AlCalzone
Copy link
Member

FWIW, I strongly recommend not doing this on Github itself, but rather offline in VSCode:
https://zwave-js.github.io/node-zwave-js/#/config-files/contributing-files
We've got tons of help built in to make the entire process easier, including auto-formatting, warning about common mistakes, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config ⚙ Configuration issues or updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants