dxdc / homebridge-blinds

:sunrise: Homebridge Plugin to control my blinds over HTTP
https://www.npmjs.com/package/homebridge-blinds
ISC License
54 stars 25 forks source link

Add an ability to define different times according to the moving direction of the blinds #41

Closed slavikme closed 4 years ago

slavikme commented 4 years ago

Added additional two configuration parameters motion_down_time and motion_up_time.

The reason for this change, is that there are some blinds that their opening time is different from closing time, due to simple physics - The weight of the shutter affects on the speed of the motor.

peros550 commented 4 years ago

I welcome this change, and I would like to mention something as a further improvement.

Let’s assume we have a closed aluminum roller shutter. Because it is 3m height, sending a open command , it would need at least 2-4 seconds before the lower edge start to move upwards. This is by design due to how roller shutters work.

Therefore: when roller shutter was completely closed & when requesting an open movement,

there should a period added to the overall time in which the motor was moving but not actual movement happened to the shutter , hence no change in the plugin’s internal position.

Otherwise, there will be always a small error in the calculation of the actual position.

At this point, I do understand that blinds, or curtains may not have this issue , because the moment motor start to operate position immediately changes.

Of course adding the ability to have different time periods is a move to the right direction, I’m just proposing something for a further improvement for people who use this plugin in combination with rolling shutters.

Many thanks

dxdc commented 4 years ago

Thanks for this PR @slavikme. I agree, it could be a nice addition and have thought of this previously because my blinds also work like this. One reason I didn't implement it previously is that most of these shades are battery powered and the battery strength decreases over time.

Overall the code looks good I will make a few notes.

I think it would be simpler to just provide the user a multiplier for the difference in one direction.

The number of config parameters is growing so have wanted to clean it up, wondering if we can add different options (esp. in the config schema) so that users are not overwhelmed. Also, at least giving the user an option for an equation could be very interesting as you point out.

@peros550

there should a period added to the overall time in which the motor was moving but not actual movement happened to the shutter

I already thought of this one, although the motor lag can be bundled with many other lags (e.g., time to transmit Rf codes, etc.). See response_lag in the config parameters.

slavikme commented 4 years ago

This is a start toward a more precise solution. What is certain, that this solution is much more precise than just using motion_time.

In my case (what eventually motivated me to create this PR), I have a daily automation for shutters that cover my windows at noon, in order to block the sun from getting inside the house and ruin my furniture. Then, on sunset, there is another command triggered to completely open the shutters. What happens, is that after several days, the completely open physical position of the shutter, covers half of the window and it requires me manually open it up, which is pretty annoying.

The change I made fixes this problem, and it is much easier now to find a precise time to handle the open and close motions correctly. This approach is much more user friendly than calculating the multiplier value, as suggested. All you need is just a stopwatch and a minute. :)

Calculating a time using an equation is a perfect solution, as you can completely customize your shutter’s behavior and retrieve the precise time to roll the shutter up or down, using just two variables: current position and targeted position. This should be added as an additional option, not overriding the current solution (with fixed times), as it is much more complicated to find such equations. But, what is good about the equation solution, is that we can predefine equations for many types and sizes of shutters. Which eventually can be contributed by other users. So that the final configuration would be just picking up your version of shutter from a drop down list (or something similar).

Regardless, I would also suggest to deprecate the option motion_time, as it may confuse the user.

Thanks for your attention :)

slavikme commented 4 years ago

By the way, as an immediate solution, I have created a parallel plugin from your repo, with my changes applied (hope this is fine by you). I am using it now and it works like a charm. You can test it out: https://www.npmjs.com/package/homebridge-blinds-up-down

dxdc commented 4 years ago

Thanks for your contribution @slavikme! I've pushed it as a new release.

Also, created two new issues based on this discussion:

dxdc commented 4 years ago

By the way, as an immediate solution, I have created a parallel plugin from your repo, with my changes applied (hope this is fine by you).

If you don't mind, maybe remove it now so as to not cause confusion.

slavikme commented 4 years ago

If you don't mind, maybe remove it now so as to not cause confusion.

Sure