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

fix(config): tweak z-trm6 options #6464

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Conversation

geirra
Copy link
Contributor

@geirra geirra commented Oct 27, 2023

This is mainly a cosmetic fix for HA.
These predefined options, but since we allow manual entry Home-Assistant doesn´t list the options.

@zwave-js-assistant zwave-js-assistant bot added the config ⚙ Configuration issues or updates label Oct 27, 2023
@geirra
Copy link
Contributor Author

geirra commented Oct 29, 2023

We should also consider if we should use the the more technical sensor descriptions.
Home-Assistant has some override code that switches the temp sensor depending on operation mode.

Example: https://github.com/home-assistant/core/blob/dev/homeassistant/components/zwave_js/discovery.py#L548

Did a quick discussion on this topic in zwave_dev but, but there was no clear recommendation. As the current layout is more in line with the new simple stile of zwave-js, it might be better to keep todays code.

The only benefit in HA would be that we could reuse some code, but it´s not much.

Basically if param2 should be changed to something like this.

		{
			"#": "2",
			"label": "Sensor Mode",
			"valueSize": 1,
			"minValue": 0,
			"maxValue": 5,
			"defaultValue": 1,
			"allowManualEntry": false,
			"options": [
				{
					"label": "F-Mode, Floor",
					"value": 0
				},
				{
					"label": "A-Mode, Internal",
					"value": 1
				},
.........
				{
					"label": "PWR-Mode, Power regulator",
					"value": 5
				}
			]
		},

Copy link
Member

@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.

Thanks. I also removed the min/max value entries which are now unnecessary.

As for the label question, I don't think application code should rely too much on labels, aside from Disable/Enable maybe.
I'm not able to keep track what would be broken by label changes otherwise.

@AlCalzone AlCalzone enabled auto-merge (squash) October 30, 2023 10:30
@AlCalzone AlCalzone merged commit d0d2855 into zwave-js:master Oct 30, 2023
15 checks passed
AlCalzone added a commit that referenced this pull request Oct 31, 2023
### Features
* Allow disabling the unresponsive controller recovery feature (#6480)

### Bugfixes
* Do not abort timed out `Send Data` commands twice (#6484)
* Ensure the default Basic CC values are only exposed if they should be, even with the compat `event` enabled (#6485)
* Auto-remove failed SmartStart nodes when bootstrapping times out (#6483)
* Do not attempt to poll values from nodes that are considered dead (#6470)
* Fixed an issue where the send queue was blocked when recovering controller from missed Send Data callback failed (#6473)
* Instead of restarting the driver, the serial port is now reopened if controller is still missing ACKs after soft-reset (#6477)
* Do not attempt to recover an unresponsive controller before fully initializing (#6480)

### Config file changes
* Tweak Heatit Z-TRM6 options (#6464)
* Add Ring Alarm Panic Button Gen2 (#6453)
* Update fingerprints for Vesternet devices (#6460)

### Changes under the hood
* Added a `mock-server` hook to run code after initializing mocks (#6478)
* Changed the headline in the logs from "ZWAVE-JS" to "Z-WAVE JS" (#6462)
* Lint device config files as part of CI (#6471)
* The `enableSoftReset` driver option is now deprecated in favor of `features.softReset` (#6480)
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.

None yet

2 participants