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

add EMS+ holidaymodes for RC310 #546

Closed
wants to merge 0 commits into from
Closed

add EMS+ holidaymodes for RC310 #546

wants to merge 0 commits into from

Conversation

tp1de
Copy link
Contributor

@tp1de tp1de commented Jun 7, 2022

telegram type id's 0x269 to 0x26D for holiday modes 1-5 (hm1 to hm5).

see #275

@proddy proddy requested a review from MichaelDvP June 21, 2022 08:07
@MichaelDvP
Copy link
Contributor

@proddy Sorry i can not review that. I dont have a RC300 and can not test. I dont understand the logic of these settings. There are a lot of setting functions, but no write to the bus. The text fields have a lot of brackets, but it is not json. The text contains the assignd circuits, but there are extra settings for them and the hc settings are not mapped to the hcs. And so on.
If i comment on half the new code a "why this" it does not help.
I think someone with RC300 should test if it works and if it is usefull.

@tp1de
Copy link
Contributor Author

tp1de commented Jun 22, 2022

I tried to do my best explaining the functionality in #275 - see last entry.
As explained the parameters and the assigned-to variables do not initiate any telegram.
These parameters are taken when one of the five holiday start-stop dates are changed by the change value dialog box.
The telegrams (0x269 to 0x26D) with all parameters and start-stop dates are only sent then afterwards.
(this to avoid to have this variables 5 times and to allow to have different settings per holiday-mode)

To visualize the parameters set per holiday-mode - either by ems-esp or by thermostat or km200 / App) they are visualized next to start-stop date within hm1 to hm5. --> see screenshots in #275

@proddy
Copy link
Contributor

proddy commented Jun 22, 2022

ok @MichaelDvP , I'll take a deeper look into this. I was concerned it's not following the guidelines or vision of ems-esp and I really don't like to see JSON as input strings for a value change. I'd rather create a new UI method for inputting these complex settings than force json parsing. I do appreciate the complexity and work that Thomas put into this so sure we'll find a way to accommodate this requirement,

@tp1de
Copy link
Contributor Author

tp1de commented Jun 22, 2022

@proddy just to be clear the hm1 to hm5 text strings are not JSON. I just take the first part (start-stop date) for input:
01.01.2023-01.02.2023 : {3,,2,[hc1,hc2,dhw1]}
The rest : {3,,2,[hc1,hc2,dhw1]} is ignored for input and just displayed for reference ...

@MichaelDvP
Copy link
Contributor

@tp1de yes, you have described it, but i doubt that this way is usefull.
Programming holidays is usefull if you have no access to the heating on start/end-days. But with ems-esp we have always access and it's much easier to set the heating to right mode at the time. Also with ioBroker/HA/... it's easy to setup a individual rule and schedule. The programming in thermostat is for all possibilitys a heating can do, it's a lot of parameters and the more parameters have to be set, someone makes errors or forget something to set.
But this is only my opinion and i dont have RC300. If other users like the way and it works, it's ok for me.
Iwould prefere a single string for each HM(1-5) containing all settings. It's a complicated string, but if accepted, all parameters are set correctly with asingle command. And this function is imo very rarly used.

You read the hc settings on first time the telegram is received, how to make sure that a this point all hcs are detected?
When setting a hc assign this is only stored in telegram on a date-change later, But (if i interprete the code right) the not stored and not active assign is shown in the hm-string, this will lead to errors if someone first set the date and than the assigns/modes.

PS: You should auto-format the new code after all additions/changes (Press in VSC-Editor: Shift-Alt-F).

@tp1de
Copy link
Contributor Author

tp1de commented Jun 26, 2022

Multiline is working with b3:
image

@tp1de
Copy link
Contributor Author

tp1de commented Jun 27, 2022

Multiline v2:

image

@tp1de
Copy link
Contributor Author

tp1de commented Jul 14, 2022

I will close since you are not interested.

@tp1de tp1de closed this Jul 14, 2022
@tefracky
Copy link
Contributor

Is it somewhere implemented? I am very interested in the holiday-mode for the RC300.

@tp1de
Copy link
Contributor Author

tp1de commented Jul 30, 2022

Is it somewhere implemented? I am very interested in the holiday-mode for the RC300

Yes I tested it and made a pull request. @proddy and @MichaelDvP where not in favor to implement this solution so a closed the PR. If you want to test you need to compile from my forked repository: https://github.com/tp1de/EMS-ESP32/tree/dev
But read carefully my comments and explanations from above !

P.S. I uploaded my latest b4 bin into my forked repository. You are free to test.

@tefracky
Copy link
Contributor

tefracky commented Aug 1, 2022

Is it somewhere implemented? I am very interested in the holiday-mode for the RC300

Yes I tested it and made a pull request. @proddy and @MichaelDvP where not in favor to implement this solution so a closed the PR. If you want to test you need to compile from my forked repository: https://github.com/tp1de/EMS-ESP32/tree/dev But read carefully my comments and explanations from above !

P.S. I uploaded my latest b4 bin into my forked repository. You are free to test.

Thanks a lot, it works!

@proddy proddy reopened this Aug 25, 2022
@proddy
Copy link
Contributor

proddy commented Aug 25, 2022

I'll re-look into the best way to implement this

@proddy proddy removed the request for review from MichaelDvP August 25, 2022 16:07
@proddy proddy self-assigned this Aug 25, 2022
@proddy proddy removed their assignment Sep 6, 2022
@tp1de tp1de closed this Sep 19, 2022
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.

4 participants