Zwave2mqtt: [bug] Z2M scales values by 255 -> 99, for the first dimmer in a node

Created on 15 May 2020  路  17Comments  路  Source: OpenZWave/Zwave2Mqtt

Version

Build/Run method

  • [x] Docker
  • [ ] PKG
  • [ ] Manually built (git clone - npm install - npm run build )

Zwave2Mqtt version: 3.1.0
Openzwave Version: 1.6.1115

Describe the bug
I was tracing why Z2M was changing my "set" value from 255 to 99. I'd prefer if Z2M did not scale my desired value, as I think this is what makes it hard for me to create proper integrations in HASS.

In these lines, I saw that Z2M scales the value, only if the object_id of the device is rgb_dimmer. This gave me a way out - simply change the object id in the discovery doc, so Z2M doesn't hit that branch.

The bug here is that this only applies to the first dimmer. I have multiple nodes with multiple dimmers, where the object ids are named like this. I tested it, and surely enough, if I send a 255 to any of the rgb_dimmer_N devices, the value isn't scaled. This means that the first dimmer works very differently from all other dimmers.

image

To Reproduce
N/A

Expected behavior

  • That Z2M does not scale my values in any way.
  • Is it possible to avoid scaling - is changing the object_id a supported scenario?

    • I changed my object_id, but it didn't immediately have an effect - maybe I need to restart Z2M?

    • EDIT: I restarted Z2M, and now the scaling is completely gone. So another bug perhaps?

Additional context
N/A

bug

All 17 comments

The lines above also apply to the first dimmer only. So, sending on will work differently on the first dimmer vs. the others.

@jshridha I think we added those line to support lights dimmer. Do you remember this?

Also ... the discussion we had in another issue reg. this, you mentioned, I think, that the 255 was sent to the device to go to its last state. But the code I'm seeing, never sends a 255.

I think a way to get the "go to last state with HASS" would be like this:

  • Inform HASS the scale goes from 0..99
  • Let HASS turn the node on by sending on
  • In Z2M, when receiving on, send 255

    • Z2M will get back a value update from the node which it forwards to HASS, so HASS can place its brightness slider correctly

  • In Z2M, when receiving 0..99, send that value to the node

Then you can ditch all scaling code :)

@LordMike The code certainly does send a 255. I just tested it out and hass sent z2m a 255 just like it should for an ON command. Below is what my discovery json looks like:

{
  "type": "light",
  "object_id": "dimmer",
  "discovery_payload": {
    "schema": "template",
    "brightness_template": "{{ (value_json.value / 99 * 255) | round(0) }}",
    "state_topic": "zwave/lr_ceiling/38/1/0",
    "state_template": "{{ \"off\" if value_json.value == 0 else \"on\" }}",
    "command_topic": "zwave/lr_ceiling/38/1/0/set",
    "command_on_template": "{{ ((brightness / 255 * 99) | round(0)) if brightness is defined else 255 }}",
    "command_off_template": "0",
    "device": {
      "identifiers": [
        "zwave2mqtt_0xe6ebd3e1_node10"
      ],
      "manufacturer": "GE (Jasco Products)",
      "model": "14294 In-Wall Smart Dimmer (0x3038)",
      "name": "lr_ceiling",
      "sw_version": "5.26"
    },
    "name": "lr_ceiling_dimmer",
    "unique_id": "zwave2mqtt_0xe6ebd3e1_10-38-1-0"
  },
  "discoveryTopic": "light/lr_ceiling/dimmer/config",
  "values": [
    "38-1-0"
  ],
  "persistent": false,
  "id": "light_dimmer"
}

It certainly sounds like a bug if this scaling template only gets applied to the first dimmer. That certainly should be fixed. Otherwise I think that Z2M is working correctly in that it takes the value it sends and forwards it to the node. I think that adding in a translation layer in there would be putting the responsibility of translating "on" is correctly done in the template and if you need to modify the template for your specific device, that can be done in the hass/devices.json file.

Sure, HASS sends a 255 - but the code I linked to is within Z2M. My claim is that Z2M doesn't send the z-wave node a 255 value. Whenever it receives a value, the code linked will scale it from 0..255 to 0..99. :)

You can see in the Z2M debug log, what it actually sends out - that's where I found this to begin with :)

My suggestion was, to make this all simpler, and still send the potential 255 value (if I missed something), was to take the approach I outlined. It would simplify the process (no scaling needed), and send the special values (255) in special cases. :)

@LordMike Thanks for that further explanation. I see the scaling that you're talking about now and sorry I missed what you were saying before. I actually don't have any rgb_dimmers, which is probably why I never noticed that. I'm not sure why that scaling is there to begin with. It doesn't exist for normal dimmers, so perhaps the payload processing should just match what is done for normal dimmers?

Aha!.. That explains the difference in what we're seeing in the other chat, too.. :)

@LordMike In poor words you are suggesting to remove this lines of code https://github.com/OpenZWave/Zwave2Mqtt/blob/master/lib/Gateway.js#L485-L488

Or just handle the boolean case and ignore the second if that does the scale?

In any case, there is the bug that the second (and other) dimmers, won't benefit from the same scaling. That should be solved no matter what.

But yea, I'd remove those lines, to remove scaling - in addition to that, my changes would include:

  • Tweaking any HASS docs as necessary (the value/brightness templates)
  • If you want the 255 value sent to indicate "last brightness used", handle that using the "on" value from HASS

Tweaking any HASS docs as necessary (the value/brightness templates)

Do you mean hass configurations? How should I tweak them?

If you want the 255 value sent to indicate "last brightness used", handle that using the "on" value from HASS

I don't understand sorry.

Would you mind to open a PR that explains this points, I will give you my support and my opinion once I see your idea in the code. The problem with such things is to break existing instances and I don't have any dimmer to test with hass, so we have to be careful

It _would_ break existing setups. Perhaps removing scaling is better left for a 4.0 release.

To fix the immediate bug though, the lines should be changed to something like "begins with 'rgb_dimmer'".. :)

You mean here

      if (this.discovered[valueId.value_id].object_id === 'rgb_dimmer') {

to

      if (this.discovered[valueId.value_id].object_id.startsWith('rgb_dimmer')) {

I suppose, also here:

if ((valueId.type === 'bool' || this.discovered[valueId.value_id].object_id === 'rgb_dimmer') && typeof payload === 'string') {

To

if ((valueId.type === 'bool' || this.discovered[valueId.value_id].object_id.startsWith('rgb_dimmer')) && typeof payload === 'string') {

Do I have to edit Just those lines so?

That will solve the immediate bug reg. scaling only applying to the first rgb_dimmer.

Check the open PR please @LordMike

Looks fine.

Was this page helpful?
0 / 5 - 0 ratings