sibartlett / homebridge-wink3

Homebridge plugin for wink.com
https://sibartlett.github.io/homebridge-wink3
ISC License
53 stars 20 forks source link

Shades with no Feedback (Somfy ZRTSI) #55

Open mriksman opened 6 years ago

mriksman commented 6 years ago

I made a heap of modifications to wink2 in order to fix;

But the most important fix was for my blinds. In wink2, adding custom code and local variables was easy for me to understand;

            var lastState;

            thisCharacteristic = platform.getaddCharacteristic(newAccessory, Service.WindowCovering, Characteristic.TargetPosition)
                .on('get', function (callback) {

// if the desired_state has changed, it was caused by the Wink App. update our local variable
                    if (newAccessory.context.desired_state.position != null) {
                         lastState = newAccessory.context.desired_state.position * 100;
                    }
// if desired_state 'times out' and goes back to undefined, set it back to what the lastState was.
                    else if (lastState != undefined) {
                         newAccessory.context.desired_state.position = lastState / 100;
                    }
                    callback(null, lastState);
                }.bind(newAccessory))
                .on('set', function (value, callback) {
                    lastState = value;
                    platform.winkAPI.deviceSetDesired(newAccessory.context.uuid, { "position": value / 100 }, callback);
                }.bind(newAccessory));
            platform.addAttributeUsage('position', newAccessory.context.uuid, thisCharacteristic);

            thisCharacteristic = platform.getaddCharacteristic(newAccessory, Service.WindowCovering, Characteristic.CurrentPosition)
                .on('get', function (callback) {
                    callback(null, lastState);
                }.bind(newAccessory));
            platform.addAttributeUsage('position', newAccessory.context.uuid, thisCharacteristic);

I had to create a variable 'lastState' that I updated when I used the blinds. The GET function would return that value.

How can I make a similar modification to your code? It's structured quite differently...

Thanks.

mriksman commented 6 years ago

Further to this, if I want to make changes to the code - should I do it in dist or src?

sibartlett commented 6 years ago

Hi, a couple of questions..

  1. The fixes you made, are they on GitHub somewhere for me to review?
  2. What was the issue you were experiencing with the blinds? Maybe I am misunderstanding things, but CurrentPosition and TargetPosition shouldn't always be the same value. CurrentPosition should always be the value from last_reading, whereas TargetPosition should be the value from desired_state or last_reading.
mriksman commented 6 years ago

Unfortunately I am not a Github user - I'm just dangerous enough to poke around the code on the Raspberry Pi and make changes to the code as required! Not ideal, I know, but it has worked so far...

The problem with the Somfy ZRTSI is that last_reading is always null. There is no feedback. So, I need to track the state using a local variable. The code I had in my post was what I added/modified to the index.js file of the homebridge-wink2 project from pdlove in the case 'shade': section.

I've started messing around with the files in dist/devices, but I haven't been able to work out where to put var lastState for use as a local variable that exists per instantiated shade.

sibartlett commented 6 years ago

Ah, thanks for the explanation! It makes me sad.

So what happens if you control the blinds using the Wink app? Does the bind not update in the Home app?

I'd have to make some changes to wink3 to support tracking additional state.

mriksman commented 6 years ago

As soon as you exit the Wink app and reopen it, they appear closed. My workaround on homebridge-wink2 worked great (as long as you didn't use Wink or the Somfy remote to open/close the blinds).

So you can't think of any location in shade.js when I can create a variable that will persist for the life of the object/blind?

Is there a way to write a line to the console within a get: set: command in the files so I can debug stuff as I go? In homebridge-wink2 I used to use; platform.log("Shade: " + lastState + " " + newAccessory.context.desired_state.position);

mriksman commented 6 years ago

@sibartlett Any further thoughts on how to provide a local variable to use to track state?

mriksman commented 6 years ago

I reached out to Stack Exchange, but they weren't any help. My attempts have so far failed - ES6/ES7 and export modules has thrown me. Anywhere I try and place a variable within the shades.js file seems to be shared amongst some/all of the other shades.

sibartlett commented 6 years ago

Sorry, that it took me so long to look at this. This might work...

Update AccessoryHelper.js, line 105 to this:

const value = get(accessory.context.last_reading, accessory.merged_state, accessory);

Then replace shade.js with this:

import { batteryService } from "./_shared";

export default ({ Characteristic, Service }) => {
  return {
    type: "shade",
    group: "shades",
    services: [
      {
        service: Service.WindowCovering,
        characteristics: [
          {
            characteristic: Characteristic.TargetPosition,
            get: (state, desired_state, accessory) => (desired_state.position || accessory.lastState) * 100,
            set: (value, accessory) => {
              accessory.lastState = value / 100;
              return { position: accessory.lastState };
            }
          },
          {
            characteristic: Characteristic.CurrentPosition,
            get: (state, desired_state, accessory) => (state.position || accessory.lastState) * 100
          },
          {
            characteristic: Characteristic.PositionState,
            value: Characteristic.PositionState.STOPPED
          }
        ]
      },
      batteryService({
        Characteristic,
        Service,
        notCharging: true
      })
    ]
  };
};
mriksman commented 6 years ago

(node:2105) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'lastState' of undefined

I see you're trying to use a variable accessory.lastState - but where should this variable be declared?

sibartlett commented 6 years ago

Hmm, you made the change to AccessoryHelper.js too? That's what ensures that the accessory variable is defined.

mriksman commented 6 years ago

so typing accessory.lastState should automatically define a new variable lastState in the accessoryobject? You don't need to declare it anywhere likevar accessory.lastState or anything?

I'm editing the files directly on my Pi (not via Git). I edit the accessories listed under homebridge-wink3/dist/devices/shade.js. I assume I edit the file homebridge-wink3/dist/AccessoryHelper.js and not the one located under homebridge-wink3/src?

sibartlett commented 6 years ago

Accessory is part of each method signature (the method parameters) - so they should be defined already.

The code I pasted above is the src version, I can give you the dist version here:

"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});

var _shared = require("./_shared");

exports.default = ({ Characteristic, Service }) => {
  return {
    type: "shade",
    group: "shades",
    services: [{
      service: Service.WindowCovering,
      characteristics: [{
        characteristic: Characteristic.TargetPosition,
        get: (state, desired_state, accessory) => (desired_state.position || accessory.lastState) * 100,
        set: (value, accessory) => {
          accessory.lastState = value / 100;
          return { position: accessory.lastState };
        }
      }, {
        characteristic: Characteristic.CurrentPosition,
        get: (state, desired_state, accessory) => (state.position || accessory.lastState) * 100
      }, {
        characteristic: Characteristic.PositionState,
        value: Characteristic.PositionState.STOPPED
      }]
    }, (0, _shared.batteryService)({
      Characteristic,
      Service,
      notCharging: true
    })]
  };
};

module.exports = exports["default"];
//# sourceMappingURL=shade.js.map
mriksman commented 6 years ago

Yeah I have already 'converted' between src and dist code :)

Still getting this error (node:3527) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'lastState' of undefined Is it saying accessory is undefined? Or lastStateis undefined? get: (state, desired_state, accessory) => (desired_state.position || accessory.lastState) * 100, Like - where have you declared lastState? Or don't you need to? It just magically appears...?

Coding. Is. HARD.

sibartlett commented 6 years ago

Cannot read property 'lastState' of undefined is saying that accessory is undefined.

mriksman commented 6 years ago

@sibartlett You missed the two getcalls further down in AccessoryHelper.js. Lines 139 and 140;

const oldValue = c.get(context.last_reading, mergedValues1);
const newValue = device && c.get(device.last_reading, mergedValues2);

Once I added accessoryas the third parameter, that error stopped.

I am playing around with it now. It's certainly progress! Thanks.

wrightmf commented 6 years ago

First of all, good work and THANK YOU. I just got set up yesterday with homebridge on an old gentoo server I had laying around. I set up the wink3 plugin and it polled correctly and grabbed my shades. They're Bali shades, which use a Somfy motor with z-wave. In the Home app, I can tap the blinds and they'll open/close and report status correctly. (YAY!)

I think my issue is related to issue #55. I have automated (time-based) triggers that open and close the blinds in the morning and at night. This morning, the blinds were opened automatically, but the Home app showed their status as closed. When I opened the Wink app, Wink reported them as open, so something had definitely broken down between Wink and homebridge. At one point, homebridge got into a weird state where it reported its status as "Opening...", despite the blinds having already been opened successfully.

I'll attempt to reproduce and see if I can better pinpoint the use case.

mriksman commented 6 years ago

Wink reported them as open? In my Wink, the default state is closed. Wink doesn't have state feedback for Somfy Z-Wave blinds. If you change the state in Wink, force close and reopen, the state will be back to its default state. Homebridge doesn't handle the whole 'no feedback' thing very well. Hence the issue. I have the modified shade.js and AccessoryHelper.js you could use to make it work?

wrightmf commented 6 years ago

Yes, Wink reported them as open. I'll try to run some additional tests today and see if I can get it back into the state it was in when I woke up this morning. In the meantime, I'd love to take a look at your modified code. Curious to know where you landed with it.

mriksman commented 6 years ago

Can't seem to upload......

mriksman commented 4 years ago

Would like to bump this one. Is @sibartlett still around?

I have to modify my code each time there is an update; AccessoryHelper.js

  readAccessory(accessory, get, callback) {     
//  const value = get(accessory.context.last_reading, accessory.merged_state); 
     const value = get(accessory.context.last_reading, accessory.merged_state, accessory); 

//  callback(null, value); 
    if (accessory.context.last_reading.connection) 
       callback(null, value); 
    else 
       callback("no_response"); 
  } 

  updateAccessoryState(accessory, device) { 
... 
      s.characteristics.filter(c => c.get).forEach(c => { 
//      const oldValue = c.get(context.last_reading, mergedValues1); 
//      const newValue = device && c.get(device.last_reading, mergedValues2); 
        const oldValue = c.get(context.last_reading, mergedValues1, accessory); 
        const newValue = device && c.get(device.last_reading, mergedValues2, accessory); 

 //     if (device && oldValue === newValue) { 
        if ( (device && oldValue === newValue) && (accessory.context.object_type != "shade") ) { 
          return; 
        } 

shade.js

           characteristic: Characteristic.TargetPosition, 
              get: (state, desired_state, accessory) => { 
                  return accessory.last_state * 100 
              }, 
              set: (value, accessory) => { 
                  accessory.last_state = value / 100; 
                  return { position: accessory.last_state }; 
              } 
          }, 
          { 
            characteristic: Characteristic.CurrentPosition, 
               get: (state, desired_state, accessory) => { 
                   return accessory.last_state * 100; 
               } 

I'd be grateful is this coul dbe merged to work with normal blinds. Keep in mind, that for the Somfy ZRTSI, the last_reading is always null, so should be easy to determine what needs a state variable saved and which do not.

I'd really like this state to survive a reboot of Homebridge as well. Perhaps it can be saved into persist or cachedAccessories? Or its own little file?

Thanks.