Skip to content
This repository has been archived by the owner on Apr 28, 2024. It is now read-only.

Closing door while it is opening causes unexpected behavior #53

Closed
dxdc opened this issue Jul 4, 2020 · 29 comments
Closed

Closing door while it is opening causes unexpected behavior #53

dxdc opened this issue Jul 4, 2020 · 29 comments

Comments

@dxdc
Copy link
Contributor

dxdc commented Jul 4, 2020

This is a legacy issue from liftmaster2, thanks for reviving this project @hjdhjd!

Essentially, if you do the following:

  • Click 'Open' to open door on homekit
  • While door is opening, click 'Close'

It causes strange behavior. The door ends up thinking its closed (when it's open) and I'm not sure if an additional API call is sent. I'm not sure the best way to deal with this, but at the very least, the plugin knows when it is opening or closing and maybe could prevent API calls in this state and/or re-update the plugin's opening/closing state.

At least, there could be a timeout before allowing a user to send another request and the door state could remain in "opening" or "closing" mode until then.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

@dxdc - let's park this one for later. I want to ensure everything else is working first. Thanks for reporting it!

And reviving...this plugin's been going for a few years! 😄 Just a function of time mostly.

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

appreciate it @hjdhjd ! Didn't know this plugin existed until today and all of the recent problems, have been limping along w/ liftmaster2 for some time now =)

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

@dxdc No worries. Can you also confirm that the above issue is an issue you see on 2.0? It should update the door state every 15-20 seconds or so. If you can recreate the problem on 2.0, with a timeline, I'll take a look next week.

What should happen in 2.0...you may be able to fool HomeKit temporarily, but the next time we poll for status from myQ, it should update the state of the door.

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

So, the door state does get updated in 2.0 as you point out, but the icon in HK is wrong. The icon says "Closing...". I was able to manually close it in spite of this, however. HK has funny problems with icons depending on how the setting is updated. I had some problems with it in another plugin.

[7/3/2020, 9:41:54 PM] [Garage Door] Opening Garage Door Opener. <-- pushed button in HK
[7/3/2020, 9:41:57 PM] [Garage Door] Closing Garage Door Opener. <-- pushed button in HK
[7/3/2020, 9:43:26 PM] [Garage Door] Opening Garage Door Opener. <-- this was from polling
[7/3/2020, 9:43:27 PM] [Garage Door] Closing Garage Door Opener. <-- closed

For reference, I had to do this to get the icon to work properly:

    this.service.getCharacteristic(Characteristic.CurrentPosition).updateValue(pos);
    this.service.getCharacteristic(Characteristic.TargetPosition).updateValue(pos);
    this.service.getCharacteristic(Characteristic.PositionState).updateValue(Characteristic.PositionState.STOPPED);

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

@dxdc Thanks...I'll file this away to mess around with at some point.

The Home app is incredibly finicky...a force-quit usually does the job without having to go to manual extremes...or using a different HomeKit controller app. Neither option is great, so fixing it more elegantly is always preferred.

Thanks!

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

Yeah... I found that force-quit isn't needed with the settings I mentioned, but many developers (even native ones) don't do that, so the force quit is required for those...

I think it's a lack of documentation and clarity from Apple. The PositionState is supposed to be dynamic (e.g., set based on the delta between current position and target position), but in practice -- if there is some kind of unexpected even -- the icon gets totally messed up and has to be reset manually. I have this issue with locks too.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

And blinds...since it seems we both have written blinds plugins. 😄

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

saw that! what do you think about a simple PR to allow for command-line scripts vs. urls? have added a lot of features since you first forked it.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

Was it your project I forked? It's been so long...

I'm all for it. Sadly in my home, it's all Somfy controlled, so I need to be able to communicate with a URTSI transmitter to send the signals. It's a more intricate setup than if Somfy would've just had reasonable support for HomeKit and others.

Sigh.

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

Was it your project I forked? It's been so long...

I've been running the project about 6+ months or so, the original developer zwerch said he didn't have time (or interest) to manage it anymore. It's very robust now.

Bookmarked this for now then: dxdc/homebridge-blinds#45

Are your Somfy blinds Rf controlled? If so, this product works beautifully: https://www.bondhome.io/product/bond-bridge/
Believe they have native Somfy support also. And, the blinds plugin manages this perfectly although https://github.com/aarons22/homebridge-bond is a great project in its own right :)

In case it's helpful, that Somfy device also looks very similar to an Arduino project I built to control another set of blinds:
https://forum.bondhome.io/t/rollease-acmeda-and-dooya-motorized-shades/413/43?u=dxdc

Not sure if it's any better that way, but I have a similar set up to this one also for another "proprietary" set of blinds.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

I'll take a look at Bond at some point. My concern in most of these bridge-type devices is longevity. When these guys get bought or go out of business, then what? It's unfortunately inevitable so I've tended to stay away from them and, in my old age, I focus on things that I know will be there for the long haul.

So what I use right now is a combination of a couple of these (large area to cover): https://www.somfysystems.com/en-us/products/1810872/universal-rts-interface and mated to these a couple of these Global Cache Flex devices: https://www.globalcache.com/products/flex/

Basically it gives me a direct HTTP to serial bridge. The Somfy URTSI is well documented...and away we go.

The command line scripts (see the sample I put up) lets me group the different shades together in useful ways for me. The Somfy RF format only allows one Somfy signal in the air at a time. So having two URTSIs transmit to two different shades in the same house can cause interference. It's hopelessly messy.

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

Bond can be run on a completely local API. Doesn't require cloud access, except for firmware updates and/or synchronizing with their app if you want.

See: http://docs-local.appbond.com/

It's fantastic and I've interacted with their dev team a lot over the past year. A terrific product. You can also use it to automate lighting with Rf-controlled sockets and such, ceiling fans, air conditioners, you name it. It doesn't have rolling code support or a Garage remote might be an option :)

I can send (with HK) 8+ simultaneous Rf commands to Bond and it will knock them out one at a time in rapid succession.

I hear you on the cmd line scripts for Somfy. Sounds messy indeed. Anyway, maybe I can integrate the command line scripting (feel free to take a stab if you want also). I have to refactor that entire plugin at some point also, getting a bit too large. Also contemplating conversion to TS.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

I just refactored this whole plugin to TS...it's part of what took me a bit. After you get over a bit of the learning curve around types and casting, its' straightforward. I'm becoming (but not quite there yet!) a fan.

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

@hjdhjd found (at least) what I think could be the problem. For one thing, the plugin doesn't update TargetDoorState after an API call. So, if you open the garage door manually (e.g., with the button, not HK) then TargetDoorState never gets updated. This I think is responsible for the icon issue.

Also, I believe:

 accessory.getService(hap.Service.GarageDoorOpener)
        ?.setCharacteristic(hap.Characteristic.CurrentDoorState, myQState);

is not the best way to do it. I think this is the correct way:

accessory.getService(hap.Service.GarageDoorOpener)
        ?.getCharacteristic(hap.Characteristic.CurrentDoorState).updateValue(myQState);
      // Update the state in HomeKit.
      accessory.getService(hap.Service.GarageDoorOpener)
        ?.getCharacteristic(hap.Characteristic.CurrentDoorState).updateValue(myQState);

      accessory.getService(hap.Service.GarageDoorOpener)
        ?.getCharacteristic(hap.Characteristic.TargetDoorState).updateValue(myQState);
    });

Lastly:

    // Check for any new or removed accessories from myQ.
@ -342,7 +341,8 @@ class myQPlatform implements DynamicPlatformPlugin {
      open:    hap.Characteristic.CurrentDoorState.OPEN,
      closed:  hap.Characteristic.CurrentDoorState.CLOSED,
      opening: hap.Characteristic.CurrentDoorState.OPENING,
      closing: hap.Characteristic.CurrentDoorState.CLOSING,
      stopped: hap.Characteristic.CurrentDoorState.STOPPED // this line is missing also <---
    };

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

I'm not sure what is returned from the API, so it's a bit more complicated I guess if an intermediate (e.g., opening or closing) value is returned.

In that case, maybe better to round the target state to the correct one -- either just open or closed. Something like this, although I'm not familiar with the myQ returned value

accessory.getService(hap.Service.GarageDoorOpener)
        ?.getCharacteristic(hap.Characteristic.TargetDoorState).updateValue(myQState > 1 ? hap.Characteristic.CurrentDoorState.OPEN : hap.Characteristic.CurrentDoorState.CLOSED);

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

@dxdc You love your updateValues(). The folks who maintain homebridge don't...and they're loud about it. 😄

The example you're citing is literally from the Homebridge Plugin documentation...I'm disinclined to make substantial changes without an awful lot of testing and tinkering. I'll look at it more in the next few days...I'm tapping out on this for tonight.

If you want to keep at it, please do, since you have passion here. That said, if you decide to submit a PR, please adhere to the current homebridge recommendations and conventions. I am trying to hard to tow the line on conformance here.

The myQ API will show one of these states:
open
closed
opening
closing
stopped (though I haven't validated this one yet and I'm not going to tonight!)

@dxdc
Copy link
Contributor Author

dxdc commented Jul 4, 2020

Can you point me to where updateValues is wrong? It avoids set loops, which is why you can't call it on TargetDoorState for example -- unless you're trying to simulate a user pushing a HK button.

E.g., homebridge/HAP-NodeJS#543

That being said, I still think this line should be changed? The TargetDoorState should be updated by the returned API value.

 accessory.getService(hap.Service.GarageDoorOpener)
        ?.getCharacteristic(hap.Characteristic.TargetDoorState)
        .getValue();

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 4, 2020

@dxdc I wish I could remember where I read it perusing some of the long list of issues associated with homebridge and this was called out as something to try to not do unless necessary. That said...you are quite correct, and in the light of day, yep. I intentionally didn't want to muck with opening and closing states for the time being to get this update out the door. But yeah...time to revisit.

Let me play with this.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 5, 2020

Should be fixed in 2.0.9. I went another route...:smile:

Let me know.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 5, 2020

Shows you how old I’m getting...this was the original conversation I was thinking of, and of course, I remembered it exactly backward when I was refactoring this app over the past couple of weeks. updateValue is the way to go...I need to look more closely to see how to continue to streamline the app.

homebridge/HAP-NodeJS#556

@dxdc
Copy link
Contributor Author

dxdc commented Jul 5, 2020

Glad we are on the same page 🦾

Thanks for making the changes. It looks good, I'll continue testing it and report back.

I added a PR (#67) which helps simplify some of the logic and address misc. linting issues. I copied your code format as exactly as possible. Mostly, I removed unused variables and used let/const instead of var throughout.

Lastly, I'm not sure about these three sections of code. I think they should probably use updateValue also, for the same reason. Particularly concerning is the last one, which will actually cause your garage door to change state if the target states are different!

          accessory
            .getService(hap.Service.GarageDoorOpener)!
            .setCharacteristic(hap.Characteristic.CurrentDoorState, hap.Characteristic.CurrentDoorState.OPENING);
          accessory
            .getService(hap.Service.GarageDoorOpener)!
            .setCharacteristic(hap.Characteristic.CurrentDoorState, hap.Characteristic.CurrentDoorState.CLOSING);
    accessory
      .addService(gdService)
      .setCharacteristic(hap.Characteristic.CurrentDoorState, doorCurrentState)
      .setCharacteristic(hap.Characteristic.TargetDoorState, doorTargetState)

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 5, 2020

Closing this out. Thanks.

@hjdhjd hjdhjd closed this as completed Jul 5, 2020
@hjdhjd
Copy link
Owner

hjdhjd commented Jul 5, 2020

Glad we are on the same page 🦾

Thanks for making the changes. It looks good, I'll continue testing it and report back.

I added a PR (#67) which helps simplify some of the logic and address misc. linting issues. I copied your code format as exactly as possible. Mostly, I removed unused variables and used let/const instead of var throughout.

Lastly, I'm not sure about these three sections of code. I think they should probably use updateValue also, for the same reason. Particularly concerning is the last one, which will actually cause your garage door to change state if the target states are different!

          accessory
            .getService(hap.Service.GarageDoorOpener)!
            .setCharacteristic(hap.Characteristic.CurrentDoorState, hap.Characteristic.CurrentDoorState.OPENING);
          accessory
            .getService(hap.Service.GarageDoorOpener)!
            .setCharacteristic(hap.Characteristic.CurrentDoorState, hap.Characteristic.CurrentDoorState.CLOSING);
    accessory
      .addService(gdService)
      .setCharacteristic(hap.Characteristic.CurrentDoorState, doorCurrentState)
      .setCharacteristic(hap.Characteristic.TargetDoorState, doorTargetState)

Haven’t forgotten about this...it’s on the radar, but I want to avoid making too many potentially problematic changes at once. I just pushed out a new release with new functionality (really, restoring old functionality...) and want to hold off and finish troubleshooting some other availability issues, and then revisit this. I do agree - they should be using updateValue given our discussion...I just want to minimize the number of changing variables in a release. 😄

Any other observations you have?

@dxdc
Copy link
Contributor Author

dxdc commented Jul 6, 2020

Haven’t forgotten about this...it’s on the radar, but I want to avoid making too many potentially problematic changes at once. I just pushed out a new release with new functionality (really, restoring old functionality...) and want to hold off and finish troubleshooting some other availability issues, and then revisit this. I do agree - they should be using updateValue given our discussion...I just want to minimize the number of changing variables in a release. 😄

Thx... makes sense. So far, I was just wondering about the stopped/obstructed state, and why that would report target as "open" instead of "stopped". Seems reasonable to me the target should be stopped if the current state is stopped?

Also, not sure how handling works if API can't be reached, if there is some nice way to indicate it. updateReachability doesn't work anymore I don't think, but not sure. I don't think Characteristic.StatusActive applies to garage doors either, unfortunately.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 6, 2020

Reachability is deprecated...I’ll get to it. One thing at a time. 😄

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 6, 2020

The myQ API does report stopped, but only if you manually stop it. Once you do...there’s no way to know what the door was trying to do before it stopped. Stopping the garage door is an operation you can only do (again in my testing) physically at the door control itself.

As to obstruction...it’ll show that it detected an obstruction indirectly by setting the state to autoreverse, but once it’s opened back up it just sets the state to open.

So...the sane / safe default for obstruction is definitely open, because that’s what myQ and the door opener actually do when the circumstance occurs.

For stopped, which seems to be your focus given earlier questions, it’s impossible to determine what the target state should be...and HomeKit only allows the target state to be open or closed for garage door types. There’s no stopped target state...and no sane way to tell what the original intent was pre-stoppage.

@dxdc
Copy link
Contributor Author

dxdc commented Jul 6, 2020

The myQ API does report stopped, but only if you manually stop it. Once you do...there’s no way to know what the door was trying to do before it stopped. Stopping the garage door is an operation you can only do (again in my testing) physically at the door control itself.

Makes sense you can only do it at the door control itself. Seems this would be more of an accidental safety issue if you tried to do it remotely. There are laws around this, which is why it has to beep so many times when you close remotely to notify anyone in the area.

As to obstruction...it’ll show that it detected an obstruction indirectly by setting the state to autoreverse, but once it’s opened back up it just sets the state to open.

This was a clever addition, didn't consider this one.

HomeKit only allows the target state to be open or closed for garage door types. There’s no stopped target state...and no sane way to tell what the original intent was pre-stoppage.

Good point about only allowing a binary state. If it is stopped it's probably somewhat arbitrary.

I guess the question is, if the user stops it manually, why did they do that? For me, it's to air out the garage (e.g., if it was mopped) but maintain some privacy. So closed (vs. open) seems a natural target state. But that may not be true for everyone. For obstructed, I think the same would be true for me -- e.g., if it goes into autoreverse it's because it hit something on the way down.

I think most of the safety measures work on the closing part more than opening, but you may have other cases in mind for that.

@hjdhjd
Copy link
Owner

hjdhjd commented Jul 10, 2020

Re: obstructions...you don’t really get a choice. As you pointed out, by law and design, they’re always designed to fail “safe”...and “safe” = open. You don’t know what you’re closing on top of. It’s just a motor on a door, after all. So when the mechanics or sensors detect an obstruction, they always go to the open position. That’s the easy one.

The stop case is always the hard one. See...the case you outlined above, wouldn’t occur to me. 😄 For me, the garage should be open or closed. The only time I would stop the garage door in a transition state would be to repair something or deal with a specific issue. It makes it just plain tough to deal with. That said...I’m going to play with this at some point once I finish tracking some other pesky bugs down. Cheers.

@github-actions
Copy link

This issue is locked to prevent necroposting on closed issues. Please create a new issue for related support requests, bug reports, or feature suggestions.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants