allow configuring the resume dim operating flag #411

lnr0626 · 2021-05-17T17:44:53Z

Proposed change

This exposes the ability to configure the "Resume Dim" operating flag. It configures the device to use the previous on level instead of the value of the on_level flag as the default level for a normal on command.

Additional Information

I'm unfamiliar with the code base, so still going through and making sure I've updated documentation where appropriate. I'm also unsure how to add unit tests for a change like this.

The command values were pulled from http://cache.insteon.com/pdf/INSTEON_Command_Tables_20070925a.pdf

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.
Code documentation was added where necessary

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated

lnr0626 · 2021-05-17T17:47:06Z

Still working on testing this locally, I'll update the PR once that's done

krkeegan · 2021-05-17T18:10:24Z

Hmm, I have never seen this setting before. Do I have this right? If Resume_Dim is enabled, then turning the device on from off will cause it to turn on to the level last used prior to off? Basically it kind of allows for a "dynamic local on level"? Does this also work if the device is turned on via command, (if so, that may require a bit of additional coding to save the last on state)?

What you have pushed so far looks like the right way to handle the setting of the flags. As for unit tests of what you are adding, you can look at the unit test here for simple tests related to setting device flags.

If you get farther along into tracking the on_level state check out the test here for guidance. This one gets more complicated, but it tests the various permutations.

lnr0626 · 2021-05-17T18:45:11Z

yup, that's exactly it - you can enable it manually by turning the dimmer off then pressing the set button. bit 2 (0-based) indicates if it's on or off in the get-flags response

I'm testing it out on my dimmers currently and running into an issue with the checksum for some reason:

>>>insteon-mqtt config.yaml set-flags tv-lights resume_dim=1
Commanding dimmer device 4a.c8.7b (tv-lights) cmd=set_flags
ERROR: 4a.c8.7b device NAK error: Checksum is incorrect, Message: Std: 4a.c8.7b->50.04.ca Type.DIRECT_NAK mh:0 hl:0 cmd: 20 fd
ERROR: Command failed. Checksum is incorrect

I'm going to keep looking into that later today, but figured I'd post in case you had pointers that might help debug.

As for unit tests ...

Thanks! I'll check out those and add some unit tests.

Does this also work if the device is turned on via command...

It works when using a scene without a level - i.e. enable resume_dim on a light, set your dimmer to some arbitrary level, turn the dimmer off, then send a command to enable scene 1 without a level specified. It'll return to the arbitrary level you had it set to.

Once I've added tests for the current stuff and figured out the error from above, I'll look into updating the on_level logic as well

* dev: Fix Various Flake Warnings Update Changelog add files created by tests to gitignore fix issue with hassio initial config setup

krkeegan · 2021-05-17T20:33:34Z

Hmm, the device is responding with a NAK. The log you posted didn't contain the output message, I think you have to at least set to INFO if not DEBUG to see those. However, the response back is the correct cmd1 value. I would want to check to see what the outgoing cmd2 value looks like. From your code, it should only be 0x04 or 0x05.

If the outgoing is correct, then maybe this is a sign that the device expects this to be an extended message and not a standard length message. Insteon documentation isn't always accurate, and they may have updated things since that document you found was released. You can look here for an example of an extended length set_operating_flags command. Basically, the extended bytes are all zeros.

lnr0626 · 2021-05-17T20:47:53Z

ooof, that looks like it was the issue. apparently one of the changes from I2 -> I2CS was apparently requiring a checksum for setting operating flags. The command table I linked above is from 2007 before I2CS was released and so didn't have that info in it :-(

However, that seems to fix my issue. It also explains why I was never able to get the keypadlinc 6-key/8-key config flag to work when I tried to play around with that

lnr0626 · 2021-05-17T21:50:46Z

I'll try my hand at the on_level changes as well, though if you're cool merging this as is I can do that as a separate PR

lnr0626 · 2021-05-17T22:10:53Z

do you prefer managing the changelog, or should I be updating that in PRs? the contribution guide doesn't specify

krkeegan · 2021-05-17T23:01:51Z

It doesn't matter to me. At times, it may be necessary to fix merge conflicts in the changelog file if there are multiple PRs. If you update the changelog you at least get the control your destiny and you are not subject to the whim of my poor grammer.

krkeegan · 2021-05-17T23:13:23Z

Another feature to add. It does not have to be done in this PR:

Add logic to the handle_on_off method in Device/base/Base that saves the on_level to set_meta('on_level") when resume_dim is enabled. This will require saving the resume_dim state to the meta field and then checking this state inside handle_on_off

So that when derive_on_level in DimmerBase is called it will report the correct on level. That way, HomeAssistant or whatever, knows the proper current level of the device when it is turned on. Without this, it will show up as 100% when it may actually be dimmed.

krkeegan · 2021-05-17T23:18:45Z

Oh, I see your comment about on_level earlier. Got it. I will merge this now and try it out over the next few days.

krkeegan · 2021-05-18T23:09:29Z

So this works in the sense that the command is successfully sent to my device and is ack'd by the device. However, I can't seem to get it to do anything. After setting the flag to true, I tried:

Turn on Light to Full using button
Dim light using button
Turn off light using button
turn back on using button
But it goes back on to full

I also tried

Turn on Light not full using button
Turn off light using button
turn back on using button
But it goes back on to full

Is there some step I am missing? Perhaps this device doesn't actually support this? It is a: DIMMABLE_LIGHTING (0x01): '2477D' (0x20) 'SwitchLinc Dimmer (Dual-Band)' firmware: 0x40

lnr0626 · 2021-05-18T23:13:45Z

hmm, both of those work on my DIMMABLE_LIGHTING (0x01): '2477D' (0x20) 'SwitchLinc Dimmer (Dual-Band)' firmware: 0x45

I'd expect it to work on yours, even with the earlier firmware version as it was documented in command tables from 2007. When you run get-flags, what're the operating flags showing as?

krkeegan · 2021-05-19T01:33:23Z

Some logs

 INFO Base: Device 1c.b5.87 (office) cmd: set flags
2021-05-18 18:31:11.592 INFO DimmerBase: Device 1c.b5.87 (office) enabling resume dim
2021-05-18 18:31:11.593 INFO Protocol: Write message to modem: Ext: 1c.b5.87, Type.DIRECT ext mh:0 hl:0, 20 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
2021-05-18 18:31:11.630 INFO Protocol: Read 0x62: Ext: 1c.b5.87, Type.DIRECT ext mh:0 hl:0, 20 04 ack: True 00 00 00 00 00 00 00 00 00 00 00 00 00 dc 
2021-05-18 18:31:11.823 INFO Protocol: Read 0x50: Std: 1c.b5.87->3c.4d.b9 Type.DIRECT_ACK mh:0 hl:0 cmd: 20 04
2021-05-18 18:31:11.824 UI Mqtt: Device set_flags complete

Commanding dimmer device 1c.b5.87 (office) cmd=get_flags
Device 1c.b5.87 operating flags: 00000000
Dimmer 1c.b5.87 (office) on_level: 254 (99.61%) ramp rate: 0.5s
Dimmer get_flags complete

It looks like while the device is ACKing this, the flags are not changing.

krkeegan · 2021-05-19T01:35:53Z

lol, ahh the Insteon fun. I found the glitch:

>>>insteon-mqtt config.yaml raw-command office 0x20 0x04
Commanding dimmer device 1c.b5.87 (office) cmd=raw_command
Device 1c.b5.87 raw message response: Std: 1c.b5.87->3c.4d.b9 Type.DIRECT_ACK mh:0 hl:0 cmd: 20 04
Raw Message complete


>>>insteon-mqtt config.yaml get-flags office
Commanding dimmer device 1c.b5.87 (office) cmd=get_flags
Device 1c.b5.87 operating flags: 00000100
Dimmer 1c.b5.87 (office) on_level: 254 (99.61%) ramp rate: 0.5s
Dimmer get_flags complete

Turns out my device with firmware 0x40 seems to require standard length messages, while your 0x45 requires extended length.

krkeegan · 2021-05-19T01:37:26Z

And it does work as expected on the device.

lnr0626 · 2021-05-19T01:37:50Z

that's..... fun

krkeegan · 2021-05-19T01:41:52Z

Agree, I suspect this is true for all the set-flags that are set using the 0x20 cmd. I don't know how many of those other features we have enabled. The solution is probably a filter that checks the firmware and then selects the proper msg format. We would need the same thing for all dimmable devices.

This doesn't have to be done now, it can be added to the list of issues that we can get to whenever.

lnr0626 · 2021-05-19T01:43:08Z

does your switch happen to be an i2 instead of an i2cs? or is insteon just that inconsistent?

krkeegan · 2021-05-19T04:15:04Z

Good call, it does report as i2.

krkeegan · 2021-05-19T16:44:06Z

I confirmed this works on i2cs devices. I will create a new issue to address non-i2cs devices.

allow configuring the resume dim operating flag

61d5d79

Merge branch 'dev' into resume-dim-op-flag

87875a2

* dev: Fix Various Flake Warnings Update Changelog add files created by tests to gitignore fix issue with hassio initial config setup

lnr0626 added 2 commits May 17, 2021 16:44

use extended command for set flag

f3cafeb

fix flake8 issues

4f41769

add test for resume dim

950deab

lnr0626 force-pushed the resume-dim-op-flag branch from 4bba776 to 950deab Compare May 17, 2021 20:54

lnr0626 marked this pull request as ready for review May 17, 2021 21:25

lnr0626 mentioned this pull request May 17, 2021

allow sending raw commands to devices #413

Merged

5 tasks

lnr0626 added 2 commits May 17, 2021 18:01

add resume dim docs to mqtt document

5a810a8

fix method docs

ccc25f1

krkeegan merged commit 3c9b5a9 into TD22057:dev May 17, 2021

lnr0626 deleted the resume-dim-op-flag branch May 17, 2021 23:21

This was referenced May 19, 2021

Enable Resume Dim Feature on non-I2CS Devices #415

Closed

Store Dim Level When Resume Dim is Enabled #416

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow configuring the resume dim operating flag #411

allow configuring the resume dim operating flag #411

lnr0626 commented May 17, 2021 •

edited

Loading

lnr0626 commented May 17, 2021

krkeegan commented May 17, 2021

lnr0626 commented May 17, 2021

krkeegan commented May 17, 2021

lnr0626 commented May 17, 2021

lnr0626 commented May 17, 2021

lnr0626 commented May 17, 2021

krkeegan commented May 17, 2021

krkeegan commented May 17, 2021

krkeegan commented May 17, 2021

krkeegan commented May 18, 2021

lnr0626 commented May 18, 2021 •

edited

Loading

krkeegan commented May 19, 2021

krkeegan commented May 19, 2021

krkeegan commented May 19, 2021

lnr0626 commented May 19, 2021

krkeegan commented May 19, 2021

lnr0626 commented May 19, 2021

krkeegan commented May 19, 2021

krkeegan commented May 19, 2021

allow configuring the resume dim operating flag #411

allow configuring the resume dim operating flag #411

Conversation

lnr0626 commented May 17, 2021 • edited Loading

Proposed change

Additional Information

Checklist

lnr0626 commented May 17, 2021

krkeegan commented May 17, 2021

lnr0626 commented May 17, 2021

krkeegan commented May 17, 2021

lnr0626 commented May 17, 2021

lnr0626 commented May 17, 2021

lnr0626 commented May 17, 2021

krkeegan commented May 17, 2021

krkeegan commented May 17, 2021

krkeegan commented May 17, 2021

krkeegan commented May 18, 2021

lnr0626 commented May 18, 2021 • edited Loading

krkeegan commented May 19, 2021

krkeegan commented May 19, 2021

krkeegan commented May 19, 2021

lnr0626 commented May 19, 2021

krkeegan commented May 19, 2021

lnr0626 commented May 19, 2021

krkeegan commented May 19, 2021

krkeegan commented May 19, 2021

lnr0626 commented May 17, 2021 •

edited

Loading

lnr0626 commented May 18, 2021 •

edited

Loading