Request: remove duplicated DS18x20 drivers from Tasmota.

[effelle]

Have you looked for this feature in other issues and in the wiki?

Yes, I have.

Is your feature request related to a problem? Please describe.

Tasmota has 3 different .ino used to managing DS18 temperature sensors:

• xsns_05_ds18x20_legacy.ino
• xsns_05_ds18b20.ino
• xsns_05_ds18x20.ino

The first must be enabled on compiler with USE_DS18x20_LEGACY and seems is used just for OneWire lib. The second should be enabled with USE_DS18B20, but this option is not present on my_user_config.h. The latter is enabled by default on classic, sonoff, knx, sensors, display bins and works just fine for single sensor as for OneWire configuration.
DS18x20 seems a standalone driver, not directly linked to DS18x20_LEGACY nor DS18B20 drivers.

Describe the solution you'd like

Remove the files if not needed anymore.

Describe alternatives you've considered

Leave them in place!

Additional context

Some user on Discord get confused when looking for a build necessary to manage a DS18 sensors when landing on release page (under Available Features and Sensors) because for single sensor seems they need a build with USE_DS18B20enabled that doesn't exists anymore.

[arendst]

Legacy, legacy and legacy...

in sonoff_post.h you'll find:

/*********************************************************************************************\
 * Mandatory define for DS18x20 if changed by above image selections
\*********************************************************************************************/

#if defined(USE_DS18x20) || defined(USE_DS18x20_LEGACY)
#else
#define USE_DS18B20                           // Default DS18B20 sensor needs no external library
#endif

So if a user does/did not define a DS18x20 there would always be code for a single DS18B20. In those days I couldn't remove optional sensors from the template.

Another issue was that when I released the non-legacy version people complained about the JSON message change between the two. To keep everyone happy there are still two drivers doing the same.

I think it's time to remove them now and prepare for a thunderstorm...

[effelle]

Theo,

the way xsns_05_ds18x20.ino parse the json is simple and effective:

if (json) {
if (1 == ds18x20_sensors) {
    ResponseAppend_P(PSTR(",\"%s\":{\"" D_JSON_TEMPERATURE "\":%s}"), ds18x20_types, temperature);
} else {
    char address[17];
    for (uint32_t j = 0; j < 6; j++) {
    sprintf(address+2*j, "%02X", ds18x20_sensor[index].address[6-j]);  // Skip sensor type and crc
    }
    ResponseAppend_P(PSTR(",\"%s\":{\"" D_JSON_ID "\":\"%s\",\"" D_JSON_TEMPERATURE "\":%s}"), ds18x20_types, address, temperature);
}

It cover all the user cases and I din't pulling my hair out when you changed the driver. For me is time to remove them too and I don't see any thunderstorm incoming...

[jziolkowski]

Would it be possiblefor the driver to detect if only one sensor is connected and reply as such, but for more than one reply with a JSON object? Or always reply with the object, even with one sensor?

And since I've been asked about this on discord, would you be able to add a SetOption which will determine what is the object key? Some users are fine with -1 -2 -3, but some users would prefer keying by the ID

[arendst]

Would it be possiblefor the driver to detect if only one sensor is connected and reply as such, but for more than one reply with a JSON object? Or always reply with the object, even with one sensor?

It has always been there:

        if (1 == ds18x20_sensors) {
          ResponseAppend_P(PSTR(",\"%s\":{\"" D_JSON_TEMPERATURE "\":%s}"), ds18x20_types, temperature);
        } else {
          char address[17];
          for (uint32_t j = 0; j < 6; j++) {
            sprintf(address+2*j, "%02X", ds18x20_sensor[index].address[6-j]);  // Skip sensor type and crc
          }
          ResponseAppend_P(PSTR(",\"%s\":{\"" D_JSON_ID "\":\"%s\",\"" D_JSON_TEMPERATURE "\":%s}"), ds18x20_types, address, temperature);
        }

[jziolkowski]

Oh I don't use that sensor; I only analyzed the various JSON outputs from different drivers, so I may be mixing something up

[effelle]

Just my consideration Jacek:
I was think on it too but after changing the code and adding Id to the name I got a long name not fitting well on webUI.
For example:
DS18B20-1 Temperature 21.9°C
vs
DS18B20-041471600DFF Temperature 21.9°C

[jziolkowski]

Yes, that's a fair point. But since the webui isn't (or at least shouldn't be) the primary interface for tasmota, then is this really a problem? I mean, the font could be downsized via CSS

[effelle]

And for completeness that's the difference of the Json dict:
Single sensor:
10:08:39 MQT: ban2/tele/SENSOR = {"Time":"2019-09-25T10:08:39","DS18B20":{"Temperature":22.1},"TempUnit":"C"}
multiple sensor:
13:24:48 MQT: DVES_63D985/tele/SENSOR = {"Time":"2019-09-18T13:24:48","DS18B20-1":{"Id":"000006DDD0F5","Temperature":16.6},"DS18B20-2":{"Id":"041470E982FF","Temperature":30.1},"TempUnit":"C"}

[jziolkowski]

ideally, for single sensor for consistency sake, should be also DS18B20-1. But that's just me ;)

[effelle]

That is feasible if @arendst agree I'll do a PR.

[arendst]

No need to do a PR as it will change legacy single sensor support.

It'll stay as it is now.

[jziolkowski]

One thunderstorm at a time? :) Well like I said, it's just me. The keys can be easily checked for presence so it's just a cosmetic change.