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

Added retrieving settings (0xA5) (validated on RC30N), and MQTT cmd t… #352

Merged
merged 2 commits into from
Mar 23, 2020
Merged

Added retrieving settings (0xA5) (validated on RC30N), and MQTT cmd t… #352

merged 2 commits into from
Mar 23, 2020

Conversation

ypaindaveine
Copy link
Contributor

Added retrieving settings (0xA5) (validated on RC30N), and MQTT cmd to set display, building type and language.

@proddy proddy merged commit 2545361 into emsesp:dev Mar 23, 2020
@ypaindaveine
Copy link
Contributor Author

ypaindaveine commented Mar 23, 2020 via email

@proddy
Copy link
Collaborator

proddy commented Mar 23, 2020

are you using the command line or Visual Studio Code?

@LineF
Copy link

LineF commented Mar 23, 2020

Hi,
you have to configure your local repository to have two sources: your own on github and proddy's.
Then you can "fetch" proddys changes to your local repository, merge them and push the result back to your own repo on github...

@ypaindaveine
Copy link
Contributor Author

ypaindaveine commented Mar 23, 2020 via email

@LineF
Copy link

LineF commented Mar 23, 2020

I'm using git for windows as base tool and on top of that Tortoise Git - an excellent integration of Git into the Windows environment. Push requests I create on Github...

@LineF
Copy link

LineF commented Mar 23, 2020

I don't have to copy locally. VSC and Tortoise Git all operate on the same directory.
So if I change something in VSC I directly see the differences in Tortoise Git and can commit and push it to Github

@ypaindaveine
Copy link
Contributor Author

ypaindaveine commented Mar 23, 2020 via email

@MichaelDvP
Copy link
Collaborator

I have tested this on RC35 (which is almost identical to RC30N):
(17:17:36) Sending raw: 0B 90 A5 00 20
(17:17:36) Thermostat -> Me, IBASettingsMessage(0xA5), telegram: 10 0B A5 00 00 00 00 00 FF F5 01 06 00 01 00 01 00 FF 00 01 00 05 20 05 28 FF 03 00 00 00 00 (#data=27)
in MQTT:
settings_data:
{"display":"int. temperature","language":"German","CalIntTemperature":0,"MinExtTemperature":-11,"building":"medium","clockOffset":0}
info:
Thermostat data:
Thermostat: RC35 (DeviceID: 0x10, ProductID: 86, Version: 01.18)
Thermostat time is 18:01:31 23/03/2020
Display: internal temperature
Language: German
Offset int. temperature: 0.0 K
Min ext. temperature: -11 C
Building: medium
Offset clock: 0 s

But writing seems not to work:
{"cmd":"language","data":"italian"}
gives in terminal:
Setting language to 3
but the telegram A5 is unchanged.
(18:14:45) Sending raw: 0B 90 A5 00 20
(18:14:46) Thermostat -> Me, IBASettingsMessage(0xA5), telegram: 10 0B A5 00 00 00 00 00 FF F5 01 06 00 01 00 01 00 FF 00 01 00 05 20 05 28 FF 03 00 00 00 00 (#data=27)

writing raw to offset 0 or 1 does not work, writing raw to offset 05 (min ext. temp) works.
I will test a bit more.

@ypaindaveine
Copy link
Contributor Author

ypaindaveine commented Mar 23, 2020 via email

@MichaelDvP
Copy link
Collaborator

OK, i've checked the menu settings:

  • Language: German
    RC35 has not language menu, it is always german and can not be changed.
  • Display:
    In the display only the upper line can be changed to Clock/Outdoortemp/Boilertemp/WWtemp
    It's in pos. 0x16 in the A5-telegram and differs from RC30N
  • Offset int. temperature: 0.0 K
    Can't test, i have weather controled ciruit, RC35 int. temp isn't measured in this mode.
  • Min ext. temperature: -11 C
    works, i can change it also
  • Building: medium
    works also
  • Offset clock: 0 s
    not present in RC35

@ypaindaveine
Copy link
Contributor Author

ypaindaveine commented Mar 24, 2020 via email

@MichaelDvP
Copy link
Collaborator

Hi Yves,
right, pos 22 (0x16) with value 3 (WWTemp). The values are:
0 Date&Clock
1 Outdoortemp
2 Boilertemp
3 WWTemp.
and according to manual Solar Collectortemp. if solarmodule is attached (i guess it's 4).

BTW:
i see that in MQTTCallback the setting for EMS_VALUE_IBASettings_LANG_DUTCH is missing, and TOPIC_SETTINGS_CMD_MINEXTTEMP is defined but not present for now.

@ypaindaveine
Copy link
Contributor Author

ypaindaveine commented Mar 24, 2020 via email

@MichaelDvP
Copy link
Collaborator

Hi Yves,
thank you for implementing the RC35. I've merged your code to my MyDev-branch and made some small changes to your code. Can you please check if it also fits your demands?
If you agree i can make the pull request to proddy, as i have some other enhencements.

The RC35 display is a bit strange. I can set the value and it is stored in the thermostat, but the display updates only if i do something manual on the RC30 (push info and go back to homescreen for instance). Is it the same on RC30? Or is there a commmand to trigger a displayupdate? (if i change the setpoint roomtemp the display shows "geänderte Raumtemperatur" in the lower line and updates also the upper line to the display setting)

You ask before how to sync with proddy. I'm also relative new to git and maybe it's not the best way, but it works for me: From the local commandline create an new branch:
git checkout -b newbranch
check if it is active
git branch
create a upstream
git remote add proddy https://github.com/proddy/EMS-ESP
fetch actual version
git fetch proddy
merge
git merge proddy/dev
You can also add other upstreams:
git remote add Michael https://github.com/MichaelDvP/EMS-ESP
git fetch Michael
git merge Michael/MyDev
to show the changes to your dev-branch:
git diff dev newbranch
and push to you github
git push --set-upstream origin newbranch

@ypaindaveine
Copy link
Contributor Author

ypaindaveine commented Mar 27, 2020 via email

@MichaelDvP
Copy link
Collaborator

Hi Yves,

it merged correctly your changes, but was not able to exploit these
to make diffs between your code and my latest dev (and i tried svc, git gui
and tortoise)

Have you merged to a new branch? From the git gui select Repository->Git Bash
there the command git diff branch1 branch2 works. In the gui or VS i have not found any possibility to show dffs between two branches, only diff between commits.

I am not sure that you used my latest developments (for example, receiving 0 from mqtt should be
allowed - that is the if(t) { should be removed).

I have merged your dev-repo from github, but in your code the mqtt-values have to be send as string with quotes {"cmd":"calintemp","data":"-1"} instead of number ("data":-1) as all other values in ems-esp, so i changed it. The code also works with value 0, try it.

On the int8_t changes, please hide hardcoded value (127) into a #define,

OK, done.

In my opinion, it would make more sense that you push your code once Proddy
has taken the pull request of my latest dev

Hmm, i think it's double work for proddy to handle two PRs, and you can test on RC30, I can test on RC35, so if we can agree to a single pull request ist less work for proddy and the code ist tested on both contollers.

@proddy
Copy link
Collaborator

proddy commented Mar 28, 2020

Is there anything I can do to help here?

@MichaelDvP
Copy link
Collaborator

@ypaindaveine

receiving 0 from mqtt should be allowed - that is the if(t)

you are right, i don't know why it works in my first test. I've changed now in the way to allow numbers including 0. I tested double for all values. (also fixed my bug in signed int handling and added mqtt for CalIntTemp)

@proddy
We are discussing how we can help you ;-) Do you have any preferences?

@proddy
Copy link
Collaborator

proddy commented Mar 28, 2020

LOL. I appreciate your contributions, really good stuff. I was thinking since both of you and going back and forth with pull requests I could create a special branch under EMS-ESP for this feature and add you both as contributors. Then you can checkin/checkout as you want.

Also I'm busy re-writing EMS-ESP for a version 2.0. I started from scratch and used more modern software design patterns to make it easily extensible. At some point I'll need to port your settings code over so watch out for that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@proddy proddy proddy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ypaindaveine @proddy @LineF @MichaelDvP