Skip to content
This repository has been archived by the owner on Oct 4, 2021. It is now read-only.

Support for multiple Thermostats (with multiple heating circuits) #238

Closed
proddy opened this issue Nov 10, 2019 · 48 comments
Closed

Support for multiple Thermostats (with multiple heating circuits) #238

proddy opened this issue Nov 10, 2019 · 48 comments
Labels
enhancement New feature or request

Comments

@proddy
Copy link
Collaborator

proddy commented Nov 10, 2019

Some people have more than one Thermostat to control different heating circuits.

#237 (comment)

@proddy proddy added the enhancement New feature or request label Nov 10, 2019
@martinSezman
Copy link

martinSezman commented Nov 23, 2019

I have two thermostats on EMS bus - RC310 and RC200.
RC310 can control (calculate parameters) up to four HCs.
RC200 can control only one HC (if works alone) or works as slave with RC300/RC310.
Regarding Buders documentation for multiple HC these is a need to have one master (and only one) RC300/RC310 and additional termostat for every additional HC. In this case thermostat (RC200) works as thermometer and additional control panel for some zone. RC200 doesn't real control HC2, because, calculations for all HCs are made by RC300/RC310.
So I think esp-ems should give higher priority for some detected thermostat devices like RC300, RC310,RC35,RC30 prior to RC200, RC100, RC25 and RC10. In practise if on EMS bus exist RC300, RC310,RC35 or RC30, we can ignore devices like RC200, RC100, RC25 and RC10.

Commands for all HCs control should be send to master thermostat.

@lmdecroos
Copy link

I have 2 thermostats a RC35 and a RCF20
The RC35 is the master thermostat and should be used to read out all settings.
Until version version 1.9.4b9, the gateway was always reading out the values of the RC35.
Since version 1.9.4b10, the gateway started to use my RC20 as main thermostat to readout the values. As the RC20 doesn't have all details, the information became less detailed.
With version 1.9.4b9 the gateway is choosing "randomly" the RC35 or RC20 as master, after a reboot the master could been changed.
In version: 1.9.4b17, the gateway is always choosing the wrong one :(
In case there is no logic to determine who is the master thermostat, then this should become a setting

@proddy
Copy link
Collaborator Author

proddy commented Nov 30, 2019

So how do we solve this?

  • does it make sense to support and show all the thermostats and their values in the info command as separate devices?
  • should we create an option to choose the master device id, like set thermostat_master <product id>

@martinSezman
Copy link

  • all thermostat in info - it will be very use full
  • set thermostat_master - great idea. In my opinion this parameter could have also default option "auto". "auto" could automatically use choose master - thermostat with the lowest DeviceID on EMS bus

@lmdecroos
Copy link

lmdecroos commented Dec 4, 2019 via email

@proddy
Copy link
Collaborator Author

proddy commented Dec 4, 2019

@lmdecroos that's an interesting approach. There must be something to tell which thermostat is the master. I'll have a look around and see where this may be held.

@tbr
Copy link

tbr commented Dec 15, 2019

Hi, my installation has the same issue. Two thermostats on one bus, the ems-esp takes the wrong thermostat (the slave) and as a consequence, setting the temperature does not work. My particular system has an RC200 and and RC300. The RC300 is the master that manages two circuits (floor and radiators).

Now, I made a small patch in the code (where the thermostat is detected), to check for the 'writable' flag of each detected thermostat. By selecting the writable one, the esp is able to correctly select the master thermostat on the bus... the patch (in src/ems.cpp) is minimal, and I will do a PR after some more testing.

@proddy
Copy link
Collaborator Author

proddy commented Dec 15, 2019

@tbr contributions always welcome, thanks! Although could you do this on the dev branch?

@tbr
Copy link

tbr commented Dec 17, 2019

@proddy I agree with the 'better' solution to be able to select the master as configuration. In any case, here is a PR to get things working in the meanwhile. Thank you all for this great project...

PS: I messed up some commit/branch/PR stuff first. I hope this PR is OK and without mistakes.

@martinSezman
Copy link

two thermostats RC310 and RC200, Please take a look how it looks in 1.9.5b3 WebUI: image

@tbr
Copy link

tbr commented Dec 20, 2019

On my mini-d1, 1.9.4 and later always crash somehow (after a while). It seems like a heap problem somewhere :(

@lmdecroos
Copy link

lmdecroos commented Dec 21, 2019 via email

@lmdecroos
Copy link

lmdecroos commented Dec 21, 2019 via email

@proddy
Copy link
Collaborator Author

proddy commented Dec 21, 2019

Did you ever try 1.9.4 when it was in dev/beta?

@lmdecroos
Copy link

lmdecroos commented Dec 21, 2019 via email

@proddy
Copy link
Collaborator Author

proddy commented Dec 21, 2019

ah right. I really need to fix this (or apply the patch PR)

proddy added a commit that referenced this issue Dec 30, 2019
@proddy
Copy link
Collaborator Author

proddy commented Dec 30, 2019

Implemented a new set option called set master_thermostat <product id>. Typing set master_thermostat will list available ids to use. Can someone test?

@tbr
Copy link

tbr commented Dec 30, 2019

I flashed the new build for testing. Will let you know if it works (or not). Seems to work...

Two things:

  • The help text misses the set master_thermostat command. Maybe you can add it?
  • It would be nice to add the option to the web UI?

Also, I need to restart the system after setting a new master thermostat before it gets used. I expect this is normal (and it is not a problem...).

Thanks for the work :)

@proddy
Copy link
Collaborator Author

proddy commented Dec 30, 2019

  • typing set shows you the commands. It's added.
  • In the Web UI do you mean like putting a "*" in the devices list to show which one is master?
  • I'll make a change so that it does a device rescan after the 'set' command, so no need to reboot

@martinSezman
Copy link

many thanks @proddy !
it works!

only in WebUI Setpoint Temperature, Current Temperature and Mode is not available as screen shot below
image

when info in console shows
image

@lmdecroos
Copy link

lmdecroos commented Dec 30, 2019 via email

@proddy
Copy link
Collaborator Author

proddy commented Dec 30, 2019

ah ok - I think I know the problem

@proddy
Copy link
Collaborator Author

proddy commented Dec 30, 2019

@lmdecroos nothing has changed. Best to grab the image from the github releases page, or just use the web interface to automatically upgrade to the latest dev build

@lmdecroos
Copy link

lmdecroos commented Dec 30, 2019 via email

@proddy
Copy link
Collaborator Author

proddy commented Dec 31, 2019

you should be seeing the two HCs under Thermostat when typing info. I'm not sure why it shows up as a Mixer?

@lmdecroos
Copy link

lmdecroos commented Jan 2, 2020 via email

@emsesp emsesp deleted a comment from lmdecroos Jan 2, 2020
@lmdecroos
Copy link

lmdecroos commented Jan 2, 2020 via email

@proddy
Copy link
Collaborator Author

proddy commented Jan 2, 2020

oh not good.

proddy added a commit that referenced this issue Jan 2, 2020
@lmdecroos
Copy link

lmdecroos commented Jan 3, 2020 via email

@proddy
Copy link
Collaborator Author

proddy commented Jan 3, 2020

I can't see what is causing this. I'll need a dump of the data. Or you could look at the code and try and figure it out.

@lmdecroos
Copy link

lmdecroos commented Jan 3, 2020 via email

@lmdecroos
Copy link

lmdecroos commented Jan 5, 2020 via email

@lmdecroos
Copy link

lmdecroos commented Jan 5, 2020 via email

@proddy
Copy link
Collaborator Author

proddy commented Jan 5, 2020

Looks like the logic needs to change. See line 1165 in ems.cpp: https://github.com/proddy/EMS-ESP/blob/daf24af323b3fec595614d43ad29ebe9f176d048/src/ems.cpp#L1195

Perhaps this is wrong. I only check for broadcast messages (-> All).

What you could do is add some myDebug(...) statements in to trace what is happening?

@lmdecroos
Copy link

lmdecroos commented Jan 5, 2020 via email

@proddy
Copy link
Collaborator Author

proddy commented Jan 6, 2020

@lmdecroos made some changes based on your tests - in 1.9.5b15. Hopefully now the jump from having 2 to 4 HCs is fixed. Also data should be coming in quicker as I removed the check for only broadcasts on RC35 status messages. Let me know...

@lmdecroos
Copy link

lmdecroos commented Jan 9, 2020 via email

@lmdecroos
Copy link

lmdecroos commented Jan 10, 2020 via email

@proddy
Copy link
Collaborator Author

proddy commented Jan 10, 2020

EMS-ESP should regularly fetch data from the thermostats, also at startup. For an RC35 it will send requests to telegrams 0x3D and 0x3E for HC1 and 0x47 and 0x48 for HC2. So not sure why you're not seeing any data. Can you manually test this with

thermostat read 47
thermostat read 48

and see what comes back and whether the data values are refreshed.

@lmdecroos
Copy link

lmdecroos commented Jan 10, 2020 via email

proddy added a commit that referenced this issue Jan 11, 2020
… and 'autodetect' also re-checks for new external sensors. #238
@proddy
Copy link
Collaborator Author

proddy commented Jan 31, 2020

@lmdecroos I'm looking into this issue and still don't understand the problem. As I mentioned earlier can you try

thermostat read 47
thermostat read 48

and tell me if the HC1 and HC2 values are refreshed correctly?

@proddy
Copy link
Collaborator Author

proddy commented Feb 2, 2020

HC1 and HC2 should work now. I'm refactoring all the code for v2 into C++ classes and have all ems devices inherit a base class which will enable EMS-ESP to support multiple devices, including thermostats and their heating circuits.

@proddy
Copy link
Collaborator Author

proddy commented Mar 8, 2020

multiple HCs support in 1.9.5

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants