8 Byte read extension with uint64_t brings inconsistent results
GnomiBerlin opened this issue · 30 comments
Installation specifics
Heating type: Vitotronic 200 KW2
Protocol: KW
Board: ESP8266 Wemos D1 mini
Hardware: own IR components
Symptom
I extended with an 8 Byte part for reading RAW-data "System Time" and the "on/off timer" from the different days of e.g. "Zirkulationspumpe"
code changes are at the end listed.
in the main program I used with additional related changes:
DP8Timer getAnlagenzeit("getanlagenzeit", "allgemein", **0x088E**); //Anlagenzeit 0x088E
DP8Timer getTimerZirkuSa("gettimerzirkusa", "allgemein", **0x2228**); //Zirkulationspumpe Zeiten Samstag
Only the address is different.
But on MQTT I get back:
gettimerzirkusa = FFFFFFFF94803B30
getanlagenzeit = 2092220
Is there anyone who can give an explanation for this. Both should deliver 8 Bytes result?!
DPTypes.cpp:
void conv8Timer::encode(uint8_t* out, DPValue in) {
//zum Schreiben
uint64_t tmp = in.getU64();
out[7] = tmp >> 56;
out[6] = tmp >> 48;
out[5] = tmp >> 40;
out[4] = tmp >> 32;
out[3] = tmp >> 24;
out[2] = tmp >> 16;
out[1] = tmp >> 8;
out[0] = tmp & 0xFF;
}
DPValue conv8Timer::decode(const uint8_t* in) {
//zum Lesen
uint64_t tmp = in[7] << 56 | in[6] << 48 | in[5] << 40 | in[4] << 32 | in[3] << 24 | in[2] << 16 | in[1] << 8 | in[0];
DPValue out(tmp);
return out;
}
DPTypes.hpp
class conv8Timer : public DPType {
public:
void encode(uint8_t* out, DPValue in);
DPValue decode(const uint8_t* in);
const size_t getLength() const { return 8; }
};
Datapoint.hpp
typedef Datapoint<conv8Timer> DP8Timer;
I believe 64 bit isn't in the code. where did you define getU64?
Hi,
thanks for the reply. I added into DPValue.hpp: see below
So I think 64 bit was addressed.
But I wonder, why with
DP8Timer getTimerZirkuSa("gettimerzirkusa", "allgemein", 0x2228); //Zirkulationspumpe Zeiten Samstag
--> delivers "FFFFFFFF94803B30" = 6:00-7:30,16:00-18:40,0:00-0:00,0:00-0:00
it is working and with
DP8Timer getAnlagenzeit("getanlagenzeit", "allgemein", 0x088E); //Anlagenzeit 0x088E
--> delivers "2092220" = only part of the system time 2.09.2022-hours?-minutes?-seconds?
not?
enum DPValueType {
BOOL,
UINT8_T,
UINT16_T,
UINT32_T,
**UINT64_T,**
FLOAT,
PTR
};
class DPValue {
private:
union value {
struct b_t { DPValueType type; bool value; } b;
struct u8_t { DPValueType type; uint8_t value; } u8;
struct u16_t { DPValueType type; uint16_t value; } u16;
struct u32_t { DPValueType type; uint32_t value; } u32;
**struct u64_t { DPValueType type; uint64_t value; } u64;**
value(uint32_t u32) : u32{UINT32_T, u32} {}
**value(uint64_t u64) : u64{UINT64_T, u64} {}**
explicit DPValue(uint32_t u32) : v(u32) {}
**explicit DPValue(uint64_t u64) : v(u64) {}**
**uint64_t getU64() {
if (v.b.type == UINT64_T) {
return v.u64.value;
} else {
return 0;
}
}**
**case UINT64_T:
snprintf(c, s, "%u", v.u64.value);
break;**
That seems to return a string containing the readable value, not the digital representation.
I'm on phone now so unable to write a correction.
Thanks! A hint after the call would also help ;-).
I can not find the point of the possible error.
So, the time windows can now be written correctly, This means
gettimerzirkusa = FFFFFFFF94803B30 (6:00-7:30+16:00-18:40) and also settimerzirkusa are working.
For writing only one adaption was missing for the 64 bit adaptation:
#include <stdlib.h> /* strtoull */
strtoul() --> strtoull():
if (strcmp(topic, "VITOWIFI/settimerzirkusa") == 0) {
uint64_t setTimer = strtoull(payload, NULL, 16);
DPValue value(setTimer);
VitoWiFi.writeDatapoint(setTimerZirkuSa, value); // 06000730160018300000000000000000 = FFFFFFFF93803B30
VitoWiFi.readDatapoint(getTimerZirkuSa);
}
if (strcmp(topic, "VITOWIFI/retrievetimerzirkusa") == 0) {
uint64_t setTimer = strtoull(payload, NULL, 16);
DPValue value(setTimer);
VitoWiFi.readDatapoint(getTimerZirkuSa);
}
getanlagenzeit = 11092220 still gets only 4 Bytes?? with the same routines
working on it. I'm unable to test anything though in real life though.
Could you test the 8-byte-dp branch with logging enable?
As your D1 only has 1 UART, you might need to resort to https://github.com/JoaoLopesF/RemoteDebug (or alternatives)
Thank you and I saw, that the extensions are nearly the same with other names as what I did. And you did not add your new rule into Datapoint.hpp like: typedef Datapoint DP8Timer;
Only difference: n.getU16() <-> n.getU64()
I tested without any difference in the result. I do not exactly know, what I have to do in the code (at which locations) for using this telnet debugging.
yours:
void conv8_1_Timer::encode(uint8_t* out, DPValue in) {
uint64_t tmp = in.getU16();
out[7] = tmp >> 56;
out[6] = tmp >> 48;
out[5] = tmp >> 40;
out[4] = tmp >> 32;
out[3] = tmp >> 24;
out[2] = tmp >> 16;
out[1] = tmp >> 8;
out[0] = tmp & 0xFF;
}
DPValue conv8_1_Timer::decode(const uint8_t* in) {
uint64_t tmp = in[7] << 56 |
in[6] << 48 |
in[5] << 40 |
in[4] << 32 |
in[3] << 24 |
in[2] << 16 |
in[1] << 8 |
in[0];
DPValue out(tmp);
return out;
}
my:
void conv8Timer::encode(uint8_t* out, DPValue in) {
//zum Schreiben
uint64_t tmp = in.getU64();
out[7] = tmp >> 56;
out[6] = tmp >> 48;
out[5] = tmp >> 40;
out[4] = tmp >> 32;
out[3] = tmp >> 24;
out[2] = tmp >> 16;
out[1] = tmp >> 8;
out[0] = tmp & 0xFF;
}
DPValue conv8Timer::decode(const uint8_t* in) {
//zum Lesen
uint64_t tmp = in[7] << 56 | in[6] << 48 | in[5] << 40 | in[4] << 32 | in[3] << 24 | in[2] << 16 | in[1] << 8 | in[0];
DPValue out(tmp);
return out;
}
Oh, forgot that, indeed + fixed the errors you mentioned
I'll add a small example. Give me a moment...
adjust to your needs:
you'll have to connect to your esp8266 using a telnet client (first link in google on how to install: https://petri.com/enable-telnet-client-in-windows-11-and-server-2022/). Check your esp8266's IP address to connect to via your router.
#include <Arduino.h>
#include <VitoWiFi.h>
#include <ESP8266WiFi.h>
#include <RemoteDebug.h>
#define HOST_NAME "VitoWiFi"
const char* ssid = "........";
const char* password = "........";
RemoteDebug debug;
VitoWiFi_setProtocol(P300);
DPTemp outsideTemp("outsideTemp", "boiler", 0x5525);
DPTemp boilerTemp("boilertemp", "boiler", 0x0810);
DPStat pumpStat("pump", "heating1", 0x2906);
void globalCallbackHandler(const IDatapoint& dp, DPValue value) {
debug.print(dp.getGroup());
debug.print(" - ");
debug.print(dp.getName());
debug.print(" is ");
char value_str[15] = {0};
value.getString(value_str, sizeof(value_str));
debug.println(value_str);
}
void setup() {
WiFi.begin(ssid, password);
while (WiFi.status() != WL_CONNECTED) {
delay(500);
}
debug.begin(HOST_NAME);
debug.setResetCmdEnabled(true); // Enable the reset command
debug.showColors(true); // Colors
VitoWiFi.enableLogger();
VitoWiFi.setLogger(&debug);
VitoWiFi.setGlobalCallback(globalCallbackHandler);
VitoWiFi.setup(&Serial);
}
void loop() {
static unsigned long lastMillis = 0;
if (millis() - lastMillis > 60 * 1000UL) { // read all values every 60 seconds
lastMillis = millis();
VitoWiFi.readAll();
}
VitoWiFi.loop();
debug.handle();
}In order to use the mentioned library, make sure to use version 2.1.2. Newer versions will not compile.
I used putty with Telnet and so was easier than to activate Windows telnet.
Telnet is running now but output is not helping me.
Mainly the difference for "getanlagenzeit" (only 4 Byte read) and the working "gettimerzirkusa" (8 Byte time windows for zirkulation pump) is of interest for us now.
debug.txt
I can put my current VitiWiFi_NodeMCU.ino again on dropbox for you?!
yes please. Although the error must be somewhere in the lib...
I did a small test:
uint64_t testvar = 0x1234567890123456;
Serial.printf("%llu\n", testvar);
Serial.printf("0x%016X\n", testvar);
Serial.printf("%"PRIu64"\n", testvar); The output was surprisingly this:
18446744073709551615
0x00000000FFFFFFFF
18446744073709551615
I thought 64-bit printing was included ages ago in the lib but it's broken apparently.
Since you publish your values in hex, this might be the reason why it fails.
You can workaround the issue by doing this:
(don't know where you use the timer value, but this was one spot it being used)
void DP8HexCallbackHandler(const IDatapoint& dp, DPValue value) {
//Umwandeln, und zum schluss per mqtt publish an mqtt-broker senden
//int64_t nValue = 0x123456789ABCDEF; //(int64_t)value.getU64();
int64_t nValue = (int64_t)value.getU64();
char outVal[24];
sprintf(outVal, "0x%lX%lX", (uint32_t)(nValue>>32), (uint32_t)nValue);
char outName[40] = "VITOWIFI/";
strcat(outName, dp.getName());
mqttClient.publish(outName, 1, true, outVal);
}See linked issue in the esp8266 repo. As far as I can tell, VitoWiFi does fetch 8 bytes but you should "print" it correctly. Don't know if you need the git version or just latest release of the Arduino core.
Hello, thanks for the analysis. I added your workaround and now get:
gettimerzirkusa = FFFFFFFF94803B30 --> gettimerzirkusa = 0xFFFFFFFF94803B30
getanlagenzeit = 13092220 --> getanlagenzeit = 0x013092220
I have to analyze, but just as a first feedback.
is there a position to debug, what bytes are really sent via the interface in both directions?
from the v-control 3.0 logging I saw for the system time:
23-08-2022 08:56:13 DATA TX: 1 F7 8 8E 8
23-08-2022 08:56:13 DATA RX: 20
23-08-2022 08:56:13 DATA RX: 22
23-08-2022 08:56:13 DATA RX: 8
23-08-2022 08:56:13 DATA RX: 23
23-08-2022 08:56:13 DATA RX: 2
23-08-2022 08:56:13 DATA RX: 8
23-08-2022 08:56:13 DATA RX: 31
23-08-2022 08:56:13 DATA RX: 17
23-08-2022 08:56:16 DATA RX: 5
So really 8 Byte?!
I've updated the PR: #76
It now includes full rx/tx logging for the P300 protocol and KW. The latter was not implemented.
thanks, how can I see it via the telnet debugging?
It's now part of standard logging. Make sure you use the right branch of VitoWiFi: https://github.com/bertmelis/VitoWiFi/tree/8-byte-dp
Hi, I can not use the libraries directly, as I made some changes on a private copy.
I updated all your updates but see no optolink interface debug information on the Telnet output. I used your standard OptolinkKW.cpp and OptolinkP300.cpp updates.
I updated all on dropbox what I used: https://www.dropbox.com/sh/iz97u42pn6ci3j7/AACdy5TvW8gvLrIYsngqVjoJa?dl=0
I saw, that your new OptolinkKW.cpp updates from today are files from yesterday late evening?!
This is the current debug with some extensions you also see in the code. Mainly again "gettimerzirkusa" and "getanlagenzeit"
debug.txt
(D) READ gettimerzirkusa
(D) READ 2228
(D) ok
(D) DP gettimerzirkusa succes
(D) [JoP]on_request - gettimerzirkusa is 18446744071889238832 with length 8
(D) [JoP]VITOWIFI/gettimerzirkusa - FFFFFFFF93803B30
...
(D) READ 088e
(D) ok
(D) DP getanlagenzeit succes
(D) [JoP]synchronize - getanlagenzeit is 319365664 with length 8
(D) [JoP]VITOWIFI/getanlagenzeit - 013092220
You sure the uploaded files are used for your firmware?
In the OptolinkKW.cpp file it should print the received values (see line 177)
the library was used. I tested with adding an error which was found during compilation.
Then I changed the order in the .ino file:
- VitoWiFi.setLogger(&debug); // for Telnet-debug
- VitoWiFi.enableLogger(); //VitoWiFi.disableLogger();VitoWiFi.enableLogger();
now it works and receives:
(D) READ f7088e08
(D) ok
(D) 2022091302125232
(D) DP getanlagenzeit succes
(D) [JoP]synchronize - getanlagenzeit is 319365664 with length 8
(D) [JoP]VITOWIFI/getanlagenzeit - 013092220
So really the preparation of the received 8 Bytes contains an error.
I compiled again, don't know why but now I got compiler warnings whereas they didn't show up last time.
Could you change decoding
DPValue conv8_1_Timer::decode(const uint8_t* in) {
uint64_t tmp = ((uint64_t)in[7]) << 56 |
((uint64_t)in[6]) << 48 |
((uint64_t)in[5]) << 40 |
((uint64_t)in[4]) << 32 |
((uint64_t)in[3]) << 24 |
((uint64_t)in[2]) << 16 |
((uint64_t)in[1]) << 8 |
in[0];
DPValue out(tmp);
return out;
}
I have no compiler errors and used:
DPValue conv8Timer::decode(const uint8_t* in) {
//zum Lesen
uint64_t tmp = in[7] << 56 | in[6] << 48 | in[5] << 40 | in[4] << 32 | in[3] << 24 | in[2] << 16 | in[1] << 8 | in[0];
DPValue out(tmp);
return out;
}
is the error perhaps here in DPValue.hpp due to the long long integer c-error?
case UINT64_T:
snprintf(c, s, "%llu", v.u64.value); //%lu ?? %u
break;
from below part
...
void getString(char* c, size_t s) {
switch (v.b.type) {
case BOOL:
snprintf(c, s, "%s", (v.b.value) ? "true" : "false");
break;
case UINT8_T:
snprintf(c, s, "%u", v.u8.value);
break;
case UINT16_T:
snprintf(c, s, "%u", v.u16.value);
break;
case UINT32_T:
snprintf(c, s, "%u", v.u32.value);
break;
case UINT64_T:
snprintf(c, s, "%llu", v.u64.value); //%lu ?? %u
break;
case FLOAT:
snprintf(c, s, "%.1f", v.f.value);
break;
case PTR:
for (uint8_t i = 0; i < v.raw.length; ++i) {
snprintf(c, s, "%02x", v.raw.value[i]);
c+=2;
}
break;
}
}
...
warnings, not errors
You have to change the decoding and cast to 64 bit before bitshifting. I've updated the branch.
For the other types, uint8_t is casted to int automatically whereas casting to 64bit has to be done explicitely.
it works - champagne!
Thanks for the hard work. I will test later, if writing is also working on 8 Byte fields...