Skip to content

Remove dimmer polling#183

Closed
loe wants to merge 1 commit into
OpenZWave:masterfrom
loe:remove-dimmer-polling
Closed

Remove dimmer polling#183
loe wants to merge 1 commit into
OpenZWave:masterfrom
loe:remove-dimmer-polling

Conversation

@loe

@loe loe commented Jul 10, 2019

Copy link
Copy Markdown

Remove superfluous dimmer polling. Application should schedule its own timer or utilize Target Value or SetChangeVerified.

See discussion on #160 .

@kdschlosser

Copy link
Copy Markdown
Contributor

any way I can get you to test something for me. I do completely understand the whole excessive network traffic thing. We want to minimize it as much as possible but acuracy is also of great importance.

if you can give the test code a go and see if it minimizes the traffic then this may be a good approach. This is going to check if the switch supports target_level. if it does and the target level matches the new value then there is no need to send a refresh to openzwave. if it does not match then add the timer. and also add the timer if the switch does not support it. and only add a single timer. no need to add 2 refreshes.

def set_dimmer(self, value_id, value):
    """
    The command 0x26 (COMMAND_CLASS_SWITCH_MULTILEVEL) of this node.
    Set switch to value (using value value_id).
    :param value_id: The value to retrieve state
    :type value_id: int
    :param value: The level : a value between 0-99 or 255. 255 set the level to the last value. \
    0 turn the dimmer off
    :type value: int
    """
    logger.debug(u"set_dimmer Level:%s", value)
    
    if value_id in self.get_dimmers():
        if 99 < value < 255:
            value = 99
        elif value < 0:
            value = 0
        self.values[value_id].data = value
        # Dimmers doesn't return the good level.
        # Add a Timer to refresh the value

        for val in self.values.values():
            if val.command_class == 0x26 and val.index == 9:
                if val.data != value:
                    timer1 = Timer(1, self.values[value_id].refresh)
                    timer1.start()
                break
        else:
            timer1 = Timer(1, self.values[value_id].refresh)
            timer1.start()
        
        return True

    return False

@loe
loe force-pushed the remove-dimmer-polling branch 2 times, most recently from 36af6a2 to d2f8a92 Compare July 22, 2019 20:41
@kdschlosser

Copy link
Copy Markdown
Contributor

I see you have added the code above. did it work??

@loe

loe commented Jul 22, 2019

Copy link
Copy Markdown
Author

It works, as in it does exactly what I expect, but not what I want.

This library is only compatible with the 1.4 OZW release which doesn't expose Target Level, so the condition will never be met; this results in an extra poll.

In the log below you see it does a set, then immediate get and receives 88 (ramping down from 99), this is standard OpenZWave behavior. It then fires the timer and gets another intermediate value of 51 which is this library's timer. This would be the "final" state, and it would be inaccurate. I have an application level timer that is aware of my configured "ramp time" so it will actually wait long enough before polling and you see it gets an accurate 0, and toggles my UI accordingly.

I feel strongly the polling here is the wrong place, the library does not have enough data to understand how long to wait, 1 second is just an arbitrary guess. It's also inconsistent that we do this when setting Value to 0 (off), but not when setting it to 99 (on), if it is truly useful we should do it on both "sides."

2019-07-22 15:21:52.666 Info, Node003, Value::Set - COMMAND_CLASS_SWITCH_MULTILEVEL - Level - 0 - 1 - 0
2019-07-22 15:21:52.666 Info, Node003, SwitchMultilevel::Set - Setting to level 0
2019-07-22 15:21:52.666 Info, Node003, Sending (Send) message (Callback ID=0xf9, Expected Reply=0x13) - SwitchMultilevelCmd_Set (Node=3): 0x01, 0x0a, 0x00, 0x13, 0x03, 0x03, 0x26, 0x01, 0x00, 0x25, 0xf9, 0x1d
2019-07-22 15:21:52.904 Info, Node003, Request RTT 237 Average Request RTT 237
2019-07-22 15:21:52.904 Info, Node003, Sending (Send) message (Callback ID=0xfa, Expected Reply=0x04) - SwitchMultilevelCmd_Get (Node=3): 0x01, 0x09, 0x00, 0x13, 0x03, 0x02, 0x26, 0x02, 0x25, 0xfa, 0x1f
2019-07-22 15:21:53.159 Info, Node003, Request RTT 256 Average Request RTT 246
2019-07-22 15:21:53.307 Info, Node003, Response RTT 403 Average Response RTT 394
2019-07-22 15:21:53.307 Info, Node003, Received SwitchMultiLevel report: level=88
2019-07-22 15:21:53.669 Info, mgr,     Refreshing node 3: COMMAND_CLASS_SWITCH_MULTILEVEL index = 0 instance = 1 (to confirm a reported change)
2019-07-22 15:21:53.669 Info, Node003, Sending (Send) message (Callback ID=0xfb, Expected Reply=0x04) - SwitchMultilevelCmd_Get (Node=3): 0x01, 0x09, 0x00, 0x13, 0x03, 0x02, 0x26, 0x02, 0x25, 0xfb, 0x1e
2019-07-22 15:21:53.906 Info, Node003, Request RTT 236 Average Request RTT 241
2019-07-22 15:21:54.053 Info, Node003, Response RTT 385 Average Response RTT 389
2019-07-22 15:21:54.053 Info, Node003, Received SwitchMultiLevel report: level=51
2019-07-22 15:21:59.056 Info, mgr,     Refreshing node 3: COMMAND_CLASS_SWITCH_MULTILEVEL index = 0 instance = 1 (to confirm a reported change)
2019-07-22 15:21:59.056 Info, Node003, Sending (Send) message (Callback ID=0xfc, Expected Reply=0x04) - SwitchMultilevelCmd_Get (Node=3): 0x01, 0x09, 0x00, 0x13, 0x03, 0x02, 0x26, 0x02, 0x25, 0xfc, 0x19
2019-07-22 15:21:59.293 Info, Node003, Request RTT 236 Average Request RTT 238
2019-07-22 15:21:59.441 Info, Node003, Response RTT 384 Average Response RTT 386
2019-07-22 15:21:59.441 Info, Node003, Received SwitchMultiLevel report: level=0

@loe

loe commented Jul 22, 2019

Copy link
Copy Markdown
Author

Log the proposed code that removes timers entirely.

2019-07-22 15:49:58.962 Info, Node003, Value::Set - COMMAND_CLASS_SWITCH_MULTILEVEL - Level - 0 - 1 - 0
2019-07-22 15:49:58.962 Info, Node003, SwitchMultilevel::Set - Setting to level 0
2019-07-22 15:49:58.962 Info, Node003, Sending (Send) message (Callback ID=0xdc, Expected Reply=0x13) - SwitchMultilevelCmd_Set (Node=3): 0x01, 0x0a, 0x00, 0x13, 0x03, 0x03, 0x26, 0x01, 0x00, 0x25, 0xdc, 0x38
2019-07-22 15:49:59.200 Info, Node003, Request RTT 238 Average Request RTT 239
2019-07-22 15:49:59.200 Info, Node003, Sending (Send) message (Callback ID=0xdd, Expected Reply=0x04) - SwitchMultilevelCmd_Get (Node=3): 0x01, 0x09, 0x00, 0x13, 0x03, 0x02, 0x26, 0x02, 0x25, 0xdd, 0x38
2019-07-22 15:49:59.453 Info, Node003, Request RTT 252 Average Request RTT 245
2019-07-22 15:49:59.605 Info, Node003, Response RTT 404 Average Response RTT 397
2019-07-22 15:49:59.605 Info, Node003, Received SwitchMultiLevel report: level=88
2019-07-22 15:50:04.608 Info, mgr,     Refreshing node 3: COMMAND_CLASS_SWITCH_MULTILEVEL index = 0 instance = 1 (to confirm a reported change)
2019-07-22 15:50:04.608 Info, Node003, Sending (Send) message (Callback ID=0xde, Expected Reply=0x04) - SwitchMultilevelCmd_Get (Node=3): 0x01, 0x09, 0x00, 0x13, 0x03, 0x02, 0x26, 0x02, 0x25, 0xde, 0x3b
2019-07-22 15:50:04.846 Info, Node003, Request RTT 237 Average Request RTT 241
2019-07-22 15:50:04.993 Info, Node003, Response RTT 384 Average Response RTT 390
2019-07-22 15:50:04.993 Info, Node003, Received SwitchMultiLevel report: level=0

@loe
loe force-pushed the remove-dimmer-polling branch from d2f8a92 to ce5f665 Compare July 22, 2019 22:51
@kdschlosser

Copy link
Copy Markdown
Contributor

If you build python-openzwave using the --flavor=dev switch then it should build using the 1.6.X code. which would expose that value, I may be wrong tho. I would have to go and check.

How are you running your tests? or you using python-openzwave in an application? or running it from a script?....

@loe

loe commented Jul 23, 2019

Copy link
Copy Markdown
Author

Home Assistant.

@loe

loe commented Sep 10, 2019

Copy link
Copy Markdown
Author

catping3

@kdschlosser

Copy link
Copy Markdown
Contributor

yeah yeah I'm awake and working on python-openzwave.

There is someone doing the HASS side of things to get the new python-openzwave installed and running. we are also getting rid of many bugs in python-openzwave and openzwave alike.

And that whole thing with doing a value refresh has been removed in the new version.

here is an invite link if you either want to test or want to chime in on any development aspects

https://join.slack.com/t/python-openzwave/shared_invite/enQtNzQyMzQ3MTM0MTQ1LWM0MDI5ZGRjMzQ2YjlhMzM2YzI2NjUxOGZlOTlmMjRjZTBiODQ1NTk5MjljNjIzMWVkODRhMWIzNzk2ODgyNDQ

@loe

loe commented Sep 11, 2019

Copy link
Copy Markdown
Author

Woo! I'd much rather have time spent on the "big" upgrade, I'll just maintain my patch independently and try to make some time to test the new things.

@loe loe closed this Sep 11, 2019
@kdschlosser

Copy link
Copy Markdown
Contributor

I thought you might like that. we are working on getting this going.

@matejdro

Copy link
Copy Markdown

And that whole thing with doing a value refresh has been removed in the new version.

Wait, am I going crazy or is this still in?

#Dimmers doesn't return the good level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants