john30 / ebusd

daemon for communication with eBUS heating systems
GNU General Public License v3.0
561 stars 130 forks source link

knx, docker: negative temperatures are not sent correctly to knx bus #1074

Closed mivola closed 8 months ago

mivola commented 9 months ago

Description



### Actual behavior

- incorrect temparature values when the temperature is below zero

### Expected behavior

- the value should be correct even if negative

### ebusd version

23.2

### ebusd arguments

see description

### Operating system

other

### CPU architecture

x64

### Dockerized

latest

### Hardware interface

other

### Related integration

TCP (cmdline client like ebusctl or netcat), MQTT generic, KNX

### Logs

I couldnt find any log that shows the incorrect value that is sent out to the KNX bus
c-huetter commented 9 months ago

What a relief, I have exactly the same issue :-)


Description

Negative temperature values are sent incorrectly to KNX. Positive temperature values are sent correctly.

Actual behavior

ebusd sends telegrams to knxd with incorrect values for negative temperatures:

time: 2023-12-04T09:02:20Z
value: -19.78
description: OutsideAirTemperature
dpt: 9.001
dptdesc: Temperature
event: GroupValue_Write

Expected behavior

The actual air temperature was -0.7 °C

# ebusctl read -V outsideairtemperature                        
kwl OutsideAirTemperature data=-0.7 °C [OutsideTemperature]

ebusd version

23.2

ebusd arguments

-c /etc/ebusd -d ens:/dev/ttyUSB0 --knxurl=ip:localhost --knxint=/etc/ebusd/knx.cfg

Operating system

Synology DSM Linux 4.4.302+

CPU architecture

arm64

Dockerized

same as ebusd version

Hardware interface

adapter 3.0 USB

Related integration

KNX

Logs

2023-12-04 09:02:14.139 [update notice] sent poll-read kwl OutsideAirTemperature QQ=31: -0.7

c-huetter commented 9 months ago

Hi everyone,

I have analyzed the issue reported by @mivola that negative temperatures are not sent correctly to KNX. The functions floatToInt16 and int16ToFloat that translate 2-byte floats into KNX data type 9 (and vice versa) seem to be buggy.

I have fixed the bugs in the translation functions and added a unit test for knxhandler in my fork ebusd-with-knxd. Please verify the bugfix and consider merging my pull request #1051.

Best, Christian

john30 commented 8 months ago

I can't reproduce this. the values are converted correctly, only in edge cases there might be a rounding issue, but this is not related to this. please post the hex value, the field definition and what was sent to knx for a wrong example

c-huetter commented 8 months ago

I've used the following test values from the repository knx-datapoints:

Hex Float
0x0000 0.0
0x07FF 20.47
0x6464 46039.04
0x7FFF 670760.96
0x87FF -0.01
0x8000 -20.48
0x8A24 -30.0
0xAC00 -327.68
0xC8C8 -9461.76
0xF800 -671088.64

Please check out the test cases in test_knxhandler.cpp to reproduce the bug. It would be easier to test the functions floatToInt16 and int16ToFloat if they were static.

john30 commented 8 months ago

what are the values that fail from this list with the current implementation?

c-huetter commented 8 months ago

The test case test_knxhandler.cpp returns the following values when using the original (probably buggy) functions floatToInt16 and int16ToFloat:

Correct: 0x0 == 0.00
Correct: 0x7ff == 20.47
Correct: 0x6464 == 46039.04
Incorrect: 0x7fff != 4294967296.00, should be 670760.94
Incorrect: 0x87ff != -20.47, should be -0.01
Incorrect: 0x8000 != -0.00, should be -20.48
Incorrect: 0x8a24 != -10.96, should be -30.00
Correct: 0xac00 == -327.68
Incorrect: 0xc8c8 != -1024.00, should be -9461.76
Incorrect: 0xf800 != -0.00, should be -671088.62
Correct: 0.00 == 0x0
Incorrect: 20.47 != 0x7fe, should be 0x7ff
Correct: 46039.04 == 0x6464
Incorrect: 670760.94 != 0x7ffe, should be 0x7fff
Incorrect: -0.01 != 0x8001, should be 0x87ff
Incorrect: -20.48 != 0x8c00, should be 0x8000
Incorrect: -30.00 != 0x8ddc, should be 0x8a24
Incorrect: -9461.76 != 0xcf38, should be 0xc8c8
Incorrect: -671088.62 != 0x7fff, should be 0xf800