Feature/improved heating cooling by dday4thedeceiver · Pull Request #30 · adey/bangali

dday4thedeceiver · 2018-02-12T05:17:06Z

This includes some of the same fixes you just did, in addition, I split the method into three (sorry, pretty bad diff, I know) and
*Added the central thermostat offset, optional
*Sends fewer commands to thermostat
*Disable any active heating/cooling when rules go out of scope too

There are some random ifDebugs I left in, been testing for the last few hours..

The older commit for presence sensors and announcement is required by improved adjacency.

dday4thedeceiver · 2018-02-12T05:46:32Z

Just realized need to add an atomicState lock through the parent when there is a thermostat offset, so two rooms don't try to control the thermostat at the same time..

adey · 2018-02-12T23:07:16Z

since some of the changes are duplicated do you want to rebase to the latest so there are no conflicts? dont want to edit your PR ... at least not till its merged. :-)

thanks.

dday4thedeceiver · 2018-02-13T00:48:04Z

Sure, but I need a ruling so I know how to merge :)

My thermostat offset is just a large value sufficient to trigger the hvac (- when cooling + when heating), not the delta between the room sensor and thermostat sensor, because I can't find such a consistent delta for my sensors. I store the thermostat setpoint in state and restore when the room is done.

Mine is guaranteed to work without a consistent delta, yours would issue fewer thermostat adjustments .

EDIT: Thinking a bit more about it, these are two distinct concepts and can coexist. I think I can make the description clear enough not to confuse the user. I'm testing a bugfix, then I'll update.

adey · 2018-02-13T05:27:30Z

I store the thermostat setpoint in state and restore when the room is done.

probably wouldnt do this part. its going to be confusing for users. if the thermostat setpoint was changed to cool or heat a room that is the new setpoint which resulted in the last set of cooling or heating action and should remain as is so user can see that change happened.

dday4thedeceiver · 2018-02-13T06:46:52Z

OK, saw the last comment after the push... well it's in there right now, take a look. Can make it optional or take it out entirely.. It's been running pretty good tonight btw.

The way I'm looking at it, I'm not coming up with a "proper" thermostat setting per se. Whatever the thermostat is normally programmed at is where it should be - I'm just temporarily changing it to get the room to the desired temperature.

~~"Nudging" the thermostat to a better "permanent" value could work too, but I haven't thought about it that way.~~

EDIT: Nevermind, you're right. What we want is the thermostat to stay at a level that gets the room to the desired temperature, not go back to what it was. So when we hit the desired room temperature we do:
roomThermostat.setHeatingSetpoint(roomThermostat.temperature)

Fix missing announcement options

Extract rule retrieval and hvac operation into own methods And fix device off bug, fine tune Bug fixes, simplify and add thermostat/sensor delta *Just quit if thermostat is already running *store only a single setpoint *assume thermostat is always shared, regardless of whether there is an offset *NEVER turn off a thermostat (could be catastrophic) Remove restore points 2/13: fixed page settings Prevent clobbering setpoint if thermostat is turned off manually

dday4thedeceiver

Squashed since diff was absolutely horrendous

dday4thedeceiver · 2018-02-14T05:26:37Z

            input "maintainRoomTemp", "enum", title: "Maintain room temperature?", required: false, multiple: false, defaultValue: 4,
                                                                            options: [[1:"Cool"], [2:"Heat"], [3:"Both"], [4:"Neither"]], submitOnChange: true
            if (['1', '2', '3'].contains(maintainRoomTemp))     {
+				if (personsPresence)


Just housekeeping and thermostatOffset, which is the "ensure trigger" offset.

trying to understand thermostatOffset since i dont use a central thermostat. does setting the thermostat to a lower or higher value then issuing a thermostat.cool() or thermostat.heat() not turn on the thermostat?

thanks.

To simplify, let's assume all sensors read the same at the same actual temperature or it's not possible to discern a delta as they seem to be all over the place, that's a separate concern. Let's also assume we want the area around the thermostat and the room at the same temperature, 70F. Finally, the overall assumption is, when we are heating this is the minimum temperature we want, we don't mind being a little warmer but certainly not colder.

Here's the scenario: The thermostat is at 71F and not heating but the room is at 68F. We want to heat the room +2F. With no sensor offset and no thermostat offset, we would make the setpoint 71F (with a swing of +-1), but that would do nothing.

With a thermostat offset of, say, 4F, we would make it 75F instead which would ensure triggering the thermostat. When the room reaches 71F, we turn off the thermostat by setting it to the current thermostat temperature (which would presumably be ~73F but could be whatever) unless it's already off (manually etc)

BTW, I just realized we turn off at the swing temperature (that's how most thermostats behave, although not all) but setting the thermostat to desired temperature, which is inconsistent. Committing a fix.

ok. do we really need the thermostat offset then? more settings usually tend to confuse users. why dont we use a static offset of 5 degrees and update the text on that page to note that.

also, we keep talking about this as a central thermostat but the handling seems to be on a per room basis. heres an example:
room A maintain: 68F room sensor: 65F temperature offset: 2F
room B maintain: 68F room sensor: 72F temperature offset: 5F
room C maintain: 68F room sensor: 70F temperature offset: 3F

this will keep toggling the thermostat ... that wont be good.

thanks.

Oh yeah, it doesn't really make sense on a per room basis. Sure, hard coded 5F sounds fine, since we don't really have a suitable spot for it in the parent app.

Busy with moving :) The plan sounds good. I'll probably get started on Sunday. I may submit a couple of other PRs first actually (adjacency & auto level) they are already running on my system but gotta clean up the commits...

ahh ... that can be consuming but hopefully not stressful. :-)

👍

guessing you are still tied up with the after move effects? :-) do you want me to get the thermostat related changes started? i also have an user asking for fan support for some time now so want to get that done as well for him. but was holding off till the thermostat changes were final.

sorry this is not meant to bug you.

thanks.

ok. let me start with #1 for thermostat.

right. since with any typical thermostat there is really no way to just heat or cool a room. might as well look at all rooms with that thermostat, find the averages of the rules and temperatures and use these average numbers to manage the thermostat.

one thing i am debating about this is whether i should transparently override user intent ... generally i am not for doing that. instead of finding all the rules and averaging them out would it better if and when they add the same thermostat more than one time to different rooms warn them that this may have undesirable effect ... blah ... blah ... and not transparently override it in the background.

any thoughts on this?

thanks.

A notification is needed but it can be subtle: "Shared thermostat, temperature averaging enabled"
They can figure it out from there.

dday4thedeceiver · 2018-02-14T05:28:47Z

@@ -2410,27 +2413,165 @@ def processCoolHeat()       {

 def processCoolHeat()       {


Split into three methods, setTemp() which does the actual heating/cooling, processCoolHeat for the logic and getActiveTemperatureRule() which is unchanged otherwise.

dday4thedeceiver · 2018-02-14T05:30:27Z

+        def thisRule = getRule(turnOn, 't')
+        def tempRange = thisRule.tempRange
+		def setpoint       
+        //reduced the off commands and fan commands to prevent spamming the likes of Nest, which have an API rate limiter


state variables added to prevent messing with the thermostat if not needed

why the state variable for heating active or cooling active instead of checking the current state of the thermostat to determine if heating or cooling is active?

thanks.

For activating, the state variables are redundant since we quit if HVAC is running (and I should remove the checks really), but for cooling it stops us from turning off activations from other rooms and from the thermostat itself.

will make the response to this a part of the centralized model discussion above.

dday4thedeceiver · 2018-02-14T05:33:26Z

+            state.heatingActive = false
+			if (useThermostat) {
+				currentTemp = roomThermostat.currentTemperature
+				if (roomThermostat.currentHeatingSetpoint <= currentTemp)


prevents changing setpoint if thermostat is manually turned off

dday4thedeceiver · 2018-02-14T05:34:32Z

+            if (useThermostat)      {
+				if (thermostatActive()) 
+					break													
+                setpoint = temp - thermoToTempSensor + thermostatOffset


oops, bug: should be -thermostatOffset

dday4thedeceiver · 2018-02-14T05:44:07Z

-                }
-            }
-            else if (temperature <= coolLow)
-                (useThermostat ? roomThermostat.off() : roomCoolSwitch.off())


Explicitly not turning thermostats off to prevent unintentional freezes

saw your notes up top on turning off thermostat being possibly catastrophic. could you expand on that?

thanks.

If we turn the thermostat off, and ST gets disconnected or something else happens, the thermostat could stay off - if unnoticed that could lead to pipes freezing etc. Turning off via lowering the setpoint instead is safe and works well.

ahh. you can probably tell i live in california 😄

haha! Ahh, I wish I could say the same...

dday4thedeceiver · 2018-02-14T05:55:51Z

+	return state[key]
+}
+
+private getActiveTemperatureRule()


No logic changes

adey · 2018-02-14T06:18:47Z

-                (useThermostat ? roomThermostat.off() : roomCoolSwitch.off())
-            if (['2', '3'].contains(maintainRoomTemp))
-                (useThermostat ? roomThermostat.off() : roomHeatSwitch.off())
-        }


turning off cool/heat with non-presence is required - no?

thanks.

Yes. It still does that, just had to move it down since we don't just turn the thermostat off and do more processing.

looks like it only does that now when it finds a rule? if yes, that wont work.

or am i misreading?

lines 2476-2479 take care of it when no rules match

i might be missing something but why process the rules if checkpresence is on and the person is not present?

thanks.

Nope, no reason. I just wanted to get to the full "off" routine and overlooked that. Will fix, along with making the offset constant.

dday4thedeceiver · 2018-03-10T23:16:29Z

No, that's ok. Sure, go for it - I don't want to hold things up. Still settling in, it will be at least another week before I can code anything :)

…

On Sat, Mar 10, 2018 at 4:17 PM, adey ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In smartapps/bangali/rooms-child-app.src/rooms-child-app.groovy <#30 (comment)>: > @@ -1241,18 +1241,21 @@ private pageRoomTemperature() { input "maintainRoomTemp", "enum", title: "Maintain room temperature?", required: false, multiple: false, defaultValue: 4, options: [[1:"Cool"], [2:"Heat"], [3:"Both"], [4:"Neither"]], submitOnChange: true if (['1', '2', '3'].contains(maintainRoomTemp)) { + if (personsPresence) guessing you are still tied up with the after move effects? :-) do you want me to get the thermostat related changes started? i also have an user asking for fan support for some time now so want to get that done as well for him. but was holding off till the thermostat changes were final. sorry this is not meant to bug you. thanks. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#30 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ARDXzotypuo7_FZyi4CVr5C2eEgHmuYAks5tdFDngaJpZM4SBtN-> .

dday4thedeceiver force-pushed the feature/improved-heating-cooling branch from 0a16a9e to 53ca0b9 Compare February 13, 2018 06:43

dday4thedeceiver force-pushed the feature/improved-heating-cooling branch from 1dd17bf to d689e0a Compare February 14, 2018 04:14

danspunkmeyer added 2 commits February 13, 2018 23:16

Support LANnouncer&VLCThing and presence without announcements

898d719

Fix missing announcement options

dday4thedeceiver force-pushed the feature/improved-heating-cooling branch from d689e0a to 1ab5ce0 Compare February 14, 2018 05:17

dday4thedeceiver commented Feb 14, 2018

View reviewed changes

Fix thermostat offset typo

b998195

dday4thedeceiver commented Feb 14, 2018

View reviewed changes

adey reviewed Feb 14, 2018

View reviewed changes

Set thermostat to swing temperature, not desired temperature

3897bfb

		@@ -2410,27 +2413,165 @@ def processCoolHeat() {

		def processCoolHeat() {

Conversation

dday4thedeceiver commented Feb 12, 2018

Uh oh!

dday4thedeceiver commented Feb 12, 2018

Uh oh!

adey commented Feb 12, 2018

Uh oh!

dday4thedeceiver commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adey commented Feb 13, 2018

Uh oh!

dday4thedeceiver commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dday4thedeceiver left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dday4thedeceiver Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adey Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dday4thedeceiver commented Feb 13, 2018 •

edited

Loading

dday4thedeceiver commented Feb 13, 2018 •

edited

Loading

dday4thedeceiver Feb 14, 2018 •

edited

Loading

adey Feb 15, 2018 •

edited

Loading