Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding SimpleBlinds3Accessory for control of roller motor shades #282

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

samsawyer
Copy link

@iRayanKhan — here's a PR for control of a variety of roller shade motors that report position to Tuya. I set up this branch from the current 2.0.1 release because I didn't have time to figure out what all had changed in the current beta work, though I did see that someone had done work to merge SimpleBlinds and SimpleBlinds2 — so if you'd like me to rename this some other way than SimpleBlinds3, please let me know.

@iRayanKhan
Copy link
Owner

iRayanKhan commented May 15, 2021

Thank you @samsawyer,

If you can rebase off main, that would be great.

Also, to merge the blind/shutter types, the GarageDoorAccessory file shows we distinguish in the config manufacturer characteristic what device type you want the plugin to use. So 'simpleBlinds3' in the manufacturer option would use that logic.

@samsawyer
Copy link
Author

samsawyer commented May 15, 2021

Ok, I will look at that over the next few days — you want me to rebase and submit an entirely new PR, or is there some way to update this one?

And does that mean you want the actual new logic integrated into SimpleBlindsAccessory directly rather than as a freestanding new file?

@iRayanKhan iRayanKhan self-requested a review May 15, 2021 18:40
@iRayanKhan
Copy link
Owner

I think there's a way to do rebase it without needing to make a new PR. I will look into it.

@samsawyer
Copy link
Author

Thanks — and regarding the manufacturer option logic, I generally see how to do that, but this type of device reports position smoothly, so it's considerably different than current SimpleBlinds doing estimates based on time-to-close etc, and those various parameters are useless for its logic. So I can integrate it into SimpleBlinds based on manufacturer logic, or I could call it something different (RollerShade ?), or I could scaffold out a BlindsWithPosition … or really however you want an WindowCovering accessory that does report position to Tuya and doesn't need it synthesized represented among the accessories.

Thanks again for all the work to maintain this plugin.

@iRayanKhan
Copy link
Owner

You could separate the logic in the same file, like if they select x it used x logic. You can choose the naming scheme for the blinds type.

Thanks again for all the work to maintain this plugin.

It's a community effort

@ElphaX
Copy link
Contributor

ElphaX commented May 20, 2021

Just to confirm @samsawyer are you going to adding 'type3' and creating if else statements to ensure roller shutter only functions are run correctly? Let me know if you need a hand.

Getting close to the big update! :)

@samsawyer
Copy link
Author

@ElphaX — yep, that's my plan.

Two questions:

  1. Is there any quick advice for how to get the homebridge-tuya plugin switched over to a feature branch? My earlier development was simple enough that I just did it via SFTP and then updated the Git repo on my own machine, but if I'm going to be branching off from the tip of current main, I'd appreciate pointers on the right way to get that environment set up? (I run homebridge on a Raspberry Pi.)
  2. I'll add a type3 to SimpleBlinds, but this type3 doesn't need any of the duration logic to calculate position — should I just ignore those settings in the schema, or is there a way to make their visibility dependent on what type is set when the accessory is configured?

Thanks for helping a novice contribute here.

@blastoma
Copy link

blastoma commented Jul 3, 2021

Hi! How can I install the version with this PR?

@tavicu
Copy link

tavicu commented Oct 4, 2021

Hi @iRayanKhan , any news regarding this merge?

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.

6 participants