Teknihall: Fix negative temperatures

[pilino1234]

The values were distorted, fixed by checking for unreasonably high value and subtracting 102.4°.
Discussion: https://forum.pilight.org/Thread-negative-Temperature-for?pid=16687#pid16687

[CurlyMoo]

Not sure if this is a permanent fix...

[pilino1234]

There are similar fixes in other protocols (alecto_ws1700 for example), and I doubt that people will measure temperatures above 80°… But if temperatures go below -22.4, it will be a problem again I guess.

[CurlyMoo]

I know, but we should be able to calculate what the crossover point is.

[pilino1234]

How do you propose to do so?

[CurlyMoo]

Getting new values to check when the values are incorrect. We a smaller range to be sure when it happens.

[jens-b]

Hm, at least i can put my sensor in a freezer to check?

[CurlyMoo]

Exactly, then try to reach -15 and let it raise until the pilight recognizes a valid temperature again. If we know that temperature we know when to set the fix.

[emeied]

The problem is in pilight/libs/pilight/core/binary.c function binToDecRev(). The binary values are 2's complement signed. This function have to handle this or is a secondary function is needed for signed values. The values need a sign expansion as well. Have a look at https://en.wikipedia.org/wiki/Sign_extension.

[CurlyMoo]

Binary is always unsigned. These devices often have a signed bit. Could be that this device also uses a signed bit, but that should be investigated.

[emeied]

auriol_protocol_v20.pdf page 6: " temperature This 12 bits wide 2's complement signed binary number represents the actual temperature value in 0.1 °C units"

Actually pilight is showing 201.8, real value is -2.9. (protocol: alecto_wx500)
(2^11-1).0.1 = 204.7 = 201.8 - -2.9

[emeied]

alecto_ws1700.c has already a fix:
if(temperature > 511) { temperature -= 1023;}

For auriol.c this would be:
line 74:
int temp = binToDecRev(binary, 12, 23); temperature = temp > 2047 ? (double)(temp-2047)/10 : (double)temp/10;

[CurlyMoo]

Ariol is ofc a different topic, but please, if you think you can improve on of our protocols, do a pull request (according to the guidelines).

[emeied]

Sorry I looked at the wrong file. My sensor uses the file alecto_wx500.c.
But in line 142 is a bug: temperature = (double)(binToDec(binary, 12, 22))/10.0; instead of temperature = (double)(binToDec(binary, 12, 23))/10.0;
So only 11 of the 12 bits will be used.

[CurlyMoo]

If that so, please do a PR with that fix and let's focus on Teknihall here,

[CurlyMoo]

Has this been confirmed?

[jens-b]

Am 24.05.16 um 20:17 schrieb CurlyMoo:

Has this been confirmed?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#221 (comment)

Sorry for late answer, the bugfix wich pilino1234 did works for me.

Kind regards
Jens Behmk

[jens-b]

Am 24.05.16 um 20:17 schrieb CurlyMoo:

Has this been confirmed?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#221 (comment)

Sorry, for long time no answer. The coldest temp was -15.9, pilight
shows something like : 86,5
At 16, the sensor display shows LL, and pilight shows 86,4

But with the manually compiled version, which pilino1234 posted works
correct and shows -16

I think everthing works fine with this bugfix.

[pilino1234]

So at what temperature does pilight show the correct value again when the sensor warms up?

[jens-b]

Without fix, pilight Shows the correct Temp in the Sensor is >0.
With fix everthing works well until -20 (the lowest whats my freezer does)

Von meinem iPhone gesendet

Am 29.05.2016 um 17:13 schrieb pilino1234 notifications@github.com:

So at what temperature does pilight show the correct value again when the sensor warms up?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[pilino1234]

What temperature does pilight (without the fix) show at <0, at the highest temperature that gives the wrong value?

[jens-b]

Hm, i think in the Forum of pilight i show all test i did.
Thats all i have for now.

Von meinem iPhone gesendet

Am 29.05.2016 um 17:23 schrieb pilino1234 notifications@github.com:

What temperature does pilight (without the fix) show at <0, at the highest temperature that gives the wrong value?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[ChristianKreuzberger]

Hi! I'm wondering whether we can get this merged or if something is missing?

I also have several teknihall receivers and I just got negative temperatures today. In pilight this temperature is wrong though.

Real: -0.5 °C
Pilight: 101.9

Real: -0.1 °C
Pilight: 102.3

Everything above 0.0 °C was working fine

[CurlyMoo]

@ChristianKreuzberger, can you check if you can get the sensor lower then 20C?

[ChristianKreuzberger]

I got it down to -18.5 °C (83.9 in Pilight). At least now I know the temperature of my freezer.

[CurlyMoo]

The point it that the value 800 doesn't make much sense binary wise.

[ChristianKreuzberger]

Last update: -19.7 °C (82.7 in pilight)
Is your problem that the code says

	if(temperature>800) {
		temperature -= 1024;
 	}

rather than a value that someone actually measured:

    if(temperature > 827) {
        temperature -= 1024;
    }

?

[CurlyMoo]

Not that it's 800 but that it's not a number you'd expect in binary. I would expect 512 (1000000000) or 1024 (10000000000). An other option is some bitwise logic as seen here:
#309 (comment)

[ChristianKreuzberger]

Let's apply some logic
The value we are reading has 10 bit, I've added the extra 11th bit for demonstrations (at the beginning of the bit string)

-25.6 °C = 011 0000 0000 (0768) // suggested minimum
-19.7 °C = 011 0011 1011 (0827)
-15.0 °C = 011 0110 1010 (0874) // spec minimum
-00.5 °C = 011 1111 1011 (1019)
-00.1 °C = 011 1111 1111 (1023)
-00.0 °C = 100 0000 0000 (1024 or 0000)
+01.8 °C = 000 0001 0010 (0018)
+20.0 °C = 000 1100 1000 (0200)
+60.0 °C = 010 0101 1000 (0600) // spec maximum
+76.7 °C = 010 1111 1111 (0767) // suggested maximum

For negative values, using two's complement works (which is also why subtracing -1024 leads to the correct values).
Therefore I suggest using 768 (-25.6 °C) as the absolute possible minimum, which is far below the specs (-15 °C), and 767 (76.7 °C) as the absolute possible maximum.

[CurlyMoo]

What do other think? 768 sounds indeed better.

[pilino1234]

Yes, I think this sounds reasonable. I'll update the PR to use 768 as the rollover value.

[ChristianKreuzberger]

Seems like my logical explaination isn't convincing enough.

To validate my claim I went to the danger zone and tried to literally grill one of my sensor. The highest temperature shown on my stations display was this:
69.9 °C (699), binary value: 10 1011 1011

The last reading I got from pilight was 75.0 °C (750), binary value: 10 1110 1110, though at this point the display is only showing HH as the temperature. I didn't want to risk my sensor dieing, so I stopped the experiment.

Considering I already got the temperature of the sensor down to -19.7 °C (11 0011 1011) and up to 75.0 °C (10 1110 1110) I believe that using 76.7 °C (10 1111 1111) as the upper bound and and -25.6 °C (11 0000 0000) as the lower bound is reasonable.

For sanity reasons they could have used a more symmetrical representation (e.g., two complement), and have a range of -(2^9)+1 = -511 to +(2^9) = 512 (thus -51.1 °C to 51.2 °C). I guess they decided against the two complement as the sensors range is at least between -19 °C to +70 °C (spec says -15 °C to +60 °C). At least from engineering perspective this makes sense, as the circuit logic could look like this:

if first two bits = 11:
    use two complement to calculate the correct negative number
else:
    display the number as is

[CurlyMoo]

Thanks alot @ChristianKreuzberger!

[pilino1234]

So I'll change the implementation to check whether the first two bits are 11, and if they are, subtract 1024 from the temperature to obtain the negative value.

[puuu]

Also the description could be possible, it sounds a very inconvenient.
Why should someone develop such a strange protocol? Only to save one bit??

If I had a short look to the wiki and the forum (Protocol Development) I really wonder, why should be the temperature from bit 14 to 23?
Why not starting from 13 or even 12. In the examples bit 12 and 13 is always 0. Since there are no examples of negative temperatures, IMHO it is still possible that the temperature is a 12bit value... (12 bit temperature values are used in some protocols: alecto_ws1700, alecto_wx500, auriol, tcm, tfa).

@ChristianKreuzberger can you record and publish the raw code for negative temperatures?
If bit 12 and 13 is 1 for negative temperatures, it might be a 12 bit two's complement.

[anx-ckreuzberger]

I will try to record some raw data of negative values, though my experience with that is very limited as I have so much noise on the 433 Mhz band.

[pilino1234]

(offtopic: @anx-ckreuzberger https://wiki.pilight.org/doku.php/low-pass_filter helps with that.)

[ChristianKreuzberger]

@pilino1234 thx for the link. I'm not really that much into hardware and doing such things myself.
@puuu I just did some debugging, and you were right. Bit 13 actually holds the carry flag, and doing a simple conversion such as

       carry = binary[13]; // int
       temperature = binToDecRev(binary, 14, 23); // int

       if (carry == 1) {
               temperature = temperature ^ 0xFFFFFC00;  // keep the last 10 bits, set the rest to 1
       }

is sufficient to get correct values. I guess that now your binToSignedRev function could also work when using bits 13 to 23?

[ChristianKreuzberger]

Yep, I can confirm that temperature = binToSignedRev(binary, 13, 23); with binToSignedRev of PR #314 works. Essentially temperature = binToSignedRev(binary, 13, 23); is equal to

    temperature = binToDecRev(binary, 14, 23); 
    if (binary[13] == 1) {
        temperature = temperature ^ 0xFFFFFC00;  // keep the last 10 bits, set the rest to 1
     }

@CurlyMoo I guess we can wait on PR #314 to be merged and then (again) change this PR to use binToSignedRev. No more ifs and subtractions with 768 or 1024 resp. needed in the code. What do you think?

[puuu]

@ChristianKreuzberger Thanks for testing. Did you checked for positive and negative Temperatures.
Did you tested only bit 13? How about bit 12?

    temperature = binToDecRev(binary, 12, 23);
    if (binary[12] == 1) {
        temperature -= 4096;
    }

The binary operation results in the same but is only valid if int is a 32 bit value.

Comparing the teknihall and the auriol protocol, they are very similar (timing, bit positions), thought auriol has no humidity. That is why I expect that the temperature value is 12 bit long.

[ChristianKreuzberger]

I did look at bit 12 too, though it was always 0 (as far as I remember).

[ChristianKreuzberger]

Now that #314 is in, should we redo this here or should I just quickly make a new PR for it?

[CurlyMoo]

Try to redo it.

[pilino1234]

Done

[puuu]

This will not work. As discussed we need to include bit 13: temperature = binToSignedRev(binary, 13, 23);

[ChristianKreuzberger]

Completely agree, this will not work as intended, needs to be temperature = binToSignedRev(binary, 13, 23);

[emeied]

I've simply added signed binToDec(Rev) functions to binary.c:

int binToDecRevSigned(const int *binary, int s, int e) { //  0<=s<=e, binary[s(msb) .. e(lsb)]
	unsigned b = e - s; // number of bits representing the number in x
	int raw;            // sign extend this b-bit number to r
	int result;         // resulting sign-extended number
	int m = 1U << b;    // mask can be pre-computed if b is fixed
	BITS_MSB_FIRST_TO_VALUE(binary, s, e, raw);
	result = (raw ^ m) - m;
	return result;
}

The function will sign extend a variable bit number in two complement notation to a normal signed int. No more subtraction is needed anymore. It is working well in my build.
This is a optimized variant of http://graphics.stanford.edu/~seander/bithacks.html#FixedSignExtend

[pilino1234]

@emeied this exact feature already exist pilight since #314