fix home assistant discovery error for unique id "teddyCloud_Server_encode-stream_max_size"

[mraction]

In my home assistant logs, I found this message:

2024-12-13 14:39:45.220 ERROR (MainThread) [homeassistant.components.mqtt.entity] Error ''max' must be > 'min'' when processing MQTT discovery message topic: 'homeassistant/number/teddyCloud_Server/encode-stream_max_size/config', message: '{'command_topic': 'teddyCloud/encode-stream_max_size/command', 'max': -2147483648, 'device': {'hw_version': 'linux x86_64', 'model': 'teddyCloud', 'configuration_url': 'http://localhost', 'sw_version': 'v0.6.2 (203f12d)', 'manufacturer': 'Team RevvoX', 'name': 'teddyCloud - Server', 'identifiers': 'teddyCloud_Server'}, 'unique_id': 'teddyCloud_Server_encode-stream_max_size', 'min': 1048575, 'name': "encode.stream_max_size - The box may create an empty file this length for each stream. So if you have 10 streaming tonies you use, the box may block 10*240MB. The only downside is, that the box will stop after the file is full and you'll need to replace the tag onto the box. Must not be a multiply of 4096, Default: 251.658.239, so 240MB, which means around 6h.", 'availability_topic': 'teddyCloud/status', 'state_topic': 'teddyCloud/encode-stream_max_size/status'}'

Here is the formated json content:

{
   "command_topic":"teddyCloud/encode-stream_max_size/command",
   "max":-2147483648,
   "device":{
      "hw_version":"linux x86_64",
      "model":"teddyCloud",
      "configuration_url":"http://localhost",
      "sw_version":"v0.6.2 (203f12d)",
      "manufacturer":"Team RevvoX",
      "name":"teddyCloud - Server",
      "identifiers":"teddyCloud_Server"
   },
   "unique_id":"teddyCloud_Server_encode-stream_max_size",
   "min":1048575,
   "name":"encode.stream_max_size - The box may create an empty file this length for each stream. So if you have 10 streaming tonies you use, the box may block 10*240MB. The only downside is, that the box will stop after the file is full and you'll need to replace the tag onto the box. Must not be a multiply of 4096, Default: 251.658.239, so 240MB, which means around 6h.",
   "availability_topic":"teddyCloud/status",
   "state_topic":"teddyCloud/encode-stream_max_size/status"
}

As we can see, the error is correct, becaus max is a greate negative number. :-(

I searched a bit in the code, but I'm not sure which is the best solution, so I decided to create an issue in the hope that someone can fix it fast based on my research. ;-)

The bacis values are set here:

teddycloud/src/settings.c

Line 243 in 9e05ef8

OPTION_UNSIGNED("encode.stream_max_size", &settings->encode.stream_max_size, 1024 * 1024 * 40 * 6 - 1, 1024 * 1024 - 1, INT32_MAX, "Max stream filesize", "The box may create an empty file this length for each stream. So if you have 10 streaming tonies you use, the box may block 10*240MB. The only downside is, that the box will stop after the file is full and you'll need to replace the tag onto the box. Must not be a multiply of 4096, Default: 251.658.239, so 240MB, which means around 6h.", LEVEL_EXPERT)

It's stored an a uint64_t type variable by that macro.
After that it's copied into a float variable here:

teddycloud/src/mqtt.c

Line 971 in 9e05ef8

entity.max = s->max.unsigned_value;

While sending it via mqtt, it's converted to int:

teddycloud/src/home_assistant.c

Line 153 in 9e05ef8

ha_addint(json_obj, "max", ha_info->entities[pos].max);

And then into double by the following cJSON_AddNumberToObject call.

The problem is the conversion step from float to int as you can see by this small example code:

#include <stdio.h>
#include <stdint.h>

int main() {
    uint64_t start = INT32_MAX;
    float step1 = start;
    int step2 = step1;

    printf("UINT64 => %lu\n", start);
    printf("FLOAT => %f\n", step1);
    printf("INT => %d\n", step2);

    return 0;
}

Output:

UINT64 => 2147483647
FLOAT => 2147483648.000000
INT => -2147483648

I would suggest to use ha_addfloat and not ha_addint here (and also for min in the line above):

teddycloud/src/home_assistant.c

Line 153 in 9e05ef8

ha_addint(json_obj, "max", ha_info->entities[pos].max);

But I'm not sure if it's ok for every entity!

I hope my investigation is helpfull. ;-)

[henryk86]

Should be fixed by @g3gg0 : f095ee7

[mraction]

@henryk86 I think, it's not a fix - it's a new potentially bug. I will describe it in a second issue. ;-)

There is a new min/max check added - that's basically ok, but would not fix this bug, because the value is used as float and will pass the check but it's still casted into int after that.

I tested it with my small example code above and it comfirmes that:

#include <stdio.h>
#include <stdint.h>

static int32_t coerce_int32(float value, int32_t min, int32_t max)
{
    if (value >= max)
    {
        return max;
    }
    if (value <= min)
    {
        return min;
    }
    return value;
}

int main() {
    uint64_t start = INT32_MAX;
    float step1 = start;
    int32_t step1a = coerce_int32(step1, INT32_MIN, INT32_MAX);
    int step2 = step1;

    printf("UINT64 => %lu\n", start);
    printf("FLOAT => %f\n", step1);
    printf("RANGE CHECK => %d\n", step1a);
    printf("INT => %d\n", step2);

    return 0;
}

Output:

UINT64 => 2147483647
FLOAT => 2147483648.000000
RANGE CHECK => 2147483647
INT => -2147483648

[henryk86]

@g3gg0 see above

[mraction]

@henryk86 You are faster then I - I linked him also in #277 . But that's only a potentially bug for future changes. :-)