-
Notifications
You must be signed in to change notification settings - Fork 449
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
Added functionallity to control blinds that require a seperate up and down pin #12
base: master
Are you sure you want to change the base?
Conversation
modified: Arduino/Sketches/ST_Anything_AlarmPanel_ESP8266WiFi/ST_Anything_AlarmPanel_ESP8266WiFi.ino new file: Arduino/libraries/ST_Anything/EX_Blind.cpp new file: Arduino/libraries/ST_Anything/EX_Blind.h new file: devicetypes/ogiewon/child-blind.src/child-blind.groovy modified: devicetypes/ogiewon/parent-st-anything-ethernet.src/parent-st-anything-ethernet.groovy modified: devicetypes/ogiewon/parent-st-anything-thingshield.src/parent-st-anything-thingshield.groovy
…-st-anything-ethernet.groovy modified: devicetypes/ogiewon/parent-st-anything-thingshield.src/parent-st-anything-thingshield.groovy
Hi Josh. I have started to review your pull request. It looks like you definitely have followed the overall architecture of ST_Anything, which is great. Looking into the specifics, I can see exactly what you're doing to automate control of your blinds. I have some concerns about the implementation being almost too specific to a particular use case. Since you have implemented a custom capability, instead of a standard SmartThings Capability, I need to determine whether or not it makes sense to be added to ST_Anything in its current form. Also, the delay(1000) statements in your new EX_Blind class are a little worrisome as they block the microcontroller from doing anything else while they are executing. If you aren't using the microcontroller for anything else other than blind control, those commands may not hurt too much. Thinking through your requirements, I believe the current IS_DoorControl device is probably a very good choice to look at. This is usually used for a Garage Door device with one timed relay out to simulate pushing the garage door button on the wall of your garage. We could extend this device's capabilities to support an "open" and a "close" pair of digital outputs, instead of just the one pin it supports today. I like this idea the more I think about it... On the SmartThings Groovy side, only minor changes to the PARENT device handler would be needed to send an "open" and "close" command to the microcontroller (I currently assume only one function is required.) If the IS_DoorControl only has one digital output pin assigned to it, it would behave exactly like it does today. If it has two pins associated with it, one would be used for open, and the other for close. The timing would be handled internally, as it is today, without the need for the delay(1000) statements. What do you think? This would keep your window blind controller more "stock" and less custom. You would be able to use stock SmartApps to control your blinds without the need for the custom capability/attributes. This makes using Apps like CoRE and WebCoRE a little simpler, and other Apps like ActionTiles will just work. Thoughts? If you like the idea above, I could make the tweaks necessary to the code over the next few days for you to try. Of course, if I have missed any of your requirements, please let me know. |
Daniel, thanks for the reply and great work on your code so far. The reason for the delay(1000) is to simulate the momentary closure of a relay required by the somfy dry contact interface I'm using. To make it more stock maybe we could extend your code like you suggested with a window shade sub device and the option to use one pin or two as you suggested. We could eliminate the timing I added and I'll just modify on my end to make it work.
… On Jun 10, 2017, at 9:55 AM, ogiewon ***@***.***> wrote:
Hi Josh. I have started to review your pull request. It looks like you definitely have followed the overall architecture of ST_Anything, which is great. Looking into the specifics, I can see exactly what you're doing to automate control of your blinds. I have some concerns about the implementation being almost too specific to a particular use case. Since you have implemented a custom capability, instead of a standard SmartThings Capability, I need to determine whether or not it makes sense to be added to ST_Anything in its current form. Also, the delay(1000) statements in your new EX_Blind class are a little worrisome as they block the microcontroller from doing anything else while they are executing. If you aren't using the microcontroller for anything else other than blind control, those commands may not hurt too much.
Thinking through your requirements, I believe the current IS_DoorControl device is probably a very good choice to look at. This is usually used for a Garage Door device with one timed relay out to simulate pushing the garage door button on the wall of your garage. We could extend this device's capabilities to support an "open" and a "close" pair of digital outputs, instead of just the one pin it supports today. I like this idea the more I think about it...
On the SmartThings Groovy side, only minor changes to the PARENT device handler would be needed to send an "open" and "close" command to the microcontroller (I currently assume only one function is required.) If the IS_DoorControl only has one digital output pin assigned to it, it would behave exactly like it does today. If it has two pins associated with it, one would be used for open, and the other for close. The timing would be handled internally, as it is today, without the need for the delay(1000) statements.
What do you think? This would keep your window blind controller more "stock" and less custom. You would be able to use stock SmartApps to control your blinds without the need for the custom capability/attributes. This makes using Apps like CoRE and WebCoRE a little simpler, and other Apps like ActionTiles will just work.
Thoughts?
If you like the idea above, I could make the tweaks necessary to the code over the next few days for you to try. Of course, if I have missed any of your requirements, please let me know.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Josh, I completely understand the need for the delay(1000). I probably wasn't clear in my original message... My IS_DoorControl class does implement an automatic timed relay method. Here is the comments section for the IS_DoorControl device class which explains all the arguments. As you can see, the last argument is the amount of time the user wants the output held high for.
My thought is to add an optional argument to the end of this list for a second pin. If supplied by a user, the first pin would be used to "open" and the second pin would be used to "close". If the second pin argument is not supplied, the first pin would be used for both "open" and "close" events. Make sense? |
Oh, yeah that works perfect. Only thing I would have to add is the window blind capability. You should just copy the door code into my files and rename it to window shade after adding the second pin code.
… On Jun 10, 2017, at 3:17 PM, ogiewon ***@***.***> wrote:
Josh, I completely understand the need for the delay(1000). I probably wasn't clear in my original message... My IS_DoorControl class does implement an automatic timed relay method. Here is the comments section for the IS_DoorControl device class which explains all the arguments. As you can see, the last argument is the amount of time the user wants the output held high for.
// st::IS_DoorControl() constructor requires the following arguments
// - String &name - REQUIRED - the name of the object - must match the Groovy ST_Anything DeviceType tile name
// - byte pinInput - REQUIRED - the Arduino Pin to be used as a digital input
// - bool iState - REQUIRED - LOW or HIGH - determines which value indicates the interrupt is true
// - bool internalPullup - REQUIRED - true == INTERNAL_PULLUP
// - byte pinOutput - REQUIRED - the Arduino Pin to be used as a digital output
// - bool startingState - REQUIRED - the value desired for the initial state of the switch. LOW = "off", HIGH = "on"
// - bool invertLogic - REQUIRED - determines whether the Arduino Digital Output should use inverted logic
// - long delayTime - REQUIRED - the number of milliseconds to keep the output on
My thought is to add an optional argument to the end of this list for a second pin. If supplied by a user, the first pin would be used to "open" and the second pin would be used to "close". If the second pin argument is not supplied, the first pin would be used for both "open" and "close" events.
Make sense?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
…nything_CustomBlinds_ESP8266WiFi.ino modified: Arduino/libraries/ST_Anything/EX_Blind.cpp modified: devicetypes/ogiewon/child-blind.src/child-blind.groovy modified: devicetypes/ogiewon/parent-st-anything-ethernet.src/parent-st-anything-ethernet.groovy
modified: devicetypes/ogiewon/child-blind.src/child-blind.groovy modified: devicetypes/ogiewon/parent-st-anything-ethernet.src/parent-st-anything-ethernet.groovy
…nything_CustomBlinds_ESP8266WiFi.ino
Hi @ogiewon @DanielOgorchock , I am interested in implementing this same type of functionality for controlling my chicken coop door. It is wired to a linear actuator which changes direction by switching polarity. I am currently using two timedRelay outputs as child devices to control up and down, but I would prefer to have it implemented as a single 'door control' device type if possible. Do you still plan to implement an optional second digital output for this device type? Thanks! |
I'm using this to control this device:
http://www.smarthome.com/somfy-1810493-dry-contact-interface.html