Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support multiple GPIO for DS18x20 sensors. Aliases of DS18x20 optimization. #16833

Merged
merged 14 commits into from
Oct 18, 2022
3 changes: 3 additions & 0 deletions tasmota/include/tasmota.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ const uint32_t LOOP_SLEEP_DELAY = 50; // Lowest number of milliseconds to
#define XPT2046_MAXX 3895
#define XPT2046_MINY 346
#define XPT2046_MAXY 3870

// Max number GPIO for DS18x20_MULTI_GPIOs
#define MAX_DSB 4
/*********************************************************************************************\
* Enumeration
\*********************************************************************************************/
Expand Down
9 changes: 9 additions & 0 deletions tasmota/include/tasmota_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -661,9 +661,18 @@ const uint16_t kGpioNiceList[] PROGMEM = {
AGPIO(GPIO_DHT11_OUT), // Pseudo Single wire DHT11, DHT21, DHT22, AM2301, AM2302, AM2321
#endif
#ifdef USE_DS18x20
#ifdef DS18x20_MULTI_GPIOs
AGPIO(GPIO_DSB) + MAX_DSB, // Single wire DS18B20 or DS18S20
#ifdef ESP8266 // ESP32 don't support dual pin mode
AGPIO(GPIO_DSB_OUT) + MAX_DSB, // Pseudo Single wire DS18B20 or DS18S20
#endif
#else
AGPIO(GPIO_DSB), // Single wire DS18B20 or DS18S20
#ifdef ESP8266 // ESP32 don't support dual pin mode
AGPIO(GPIO_DSB_OUT), // Pseudo Single wire DS18B20 or DS18S20
#endif
#endif //DS18x20_MULTI_GPIOs
#endif
#ifdef USE_LMT01
AGPIO(GPIO_LMT01), // LMT01, count pulses on GPIO
#endif
Expand Down
3 changes: 2 additions & 1 deletion tasmota/my_user_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,8 @@
// -- One wire sensors ----------------------------
#define USE_DS18x20 // Add support for DS18x20 sensors with id sort, single scan and read retry (+2k6 code)
// #define W1_PARASITE_POWER // Optimize for parasite powered sensors
// #define DS18x20_USE_ID_ALIAS
// #define DS18x20_USE_ID_ALIAS // Add support aliasing for DS18x20 sensors. See comments in xsns_05 files (+0k5 code)
// #define DS18x20_MULTI_GPIOs // Add support multiple GPIOs for DS18x20 sensors (+0k2 code)

// -- I2C sensors ---------------------------------
#define USE_I2C // I2C using library wire (+10k code, 0k2 mem, 124 iram)
Expand Down
2 changes: 1 addition & 1 deletion tasmota/tasmota_xdrv_driver/xdrv_11_knx.ino
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ void KNX_INIT(void)
device_param[KNX_HUMIDITY-1].show = true;
}
#ifdef USE_DS18x20
if (PinUsed(GPIO_DSB)) {
if (PinUsed(GPIO_DSB, GPIO_ANY)) {
device_param[KNX_TEMPERATURE-1].show = true;
}
#endif
Expand Down
87 changes: 75 additions & 12 deletions tasmota/tasmota_xsns_sensor/xsns_05_ds18x20.ino
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
xsns_05_ds18x20.ino - DS18x20 temperature sensor support for Tasmota

Copyright (C) 2021 Theo Arends

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
Expand All @@ -15,6 +15,8 @@

You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.

Updated by md5sum-as (https://github.com/md5sum-as)
*/

#ifdef ESP8266
Expand All @@ -30,7 +32,7 @@

/* #define DS18x20_USE_ID_ALIAS in my_user_config.h or user_config_override.h
* Use alias for fixed sensor name in scripts by autoexec. Command: DS18Alias XXXXXXXXXXXXXXXX,N where XXXXXXXXXXXXXXXX full serial and N number 1-255
* Result in JSON: "DS18Alias_2":{"Id":"000003287CD8","Temperature":26.3} (example with N=2)
* Result in JSON: "DS18Sens_2":{"Id":"000003287CD8","Temperature":26.3} (example with N=2)
* add 8 bytes used memory
*/

Expand Down Expand Up @@ -63,8 +65,20 @@ struct {
#ifdef DS18x20_USE_ID_ALIAS
uint8_t alias;
#endif //DS18x20_USE_ID_ALIAS
#ifdef DS18x20_MULTI_GPIOs
int8_t pins_id = 0;
#endif //DS18x20_MULTI_GPIOs
} ds18x20_sensor[DS18X20_MAX_SENSORS];

#ifdef DS18x20_MULTI_GPIOs
struct {
int8_t pin = 0; // Shelly GPIO3 input only
int8_t pin_out = 0; // Shelly GPIO00 output only
bool dual_mode = false; // Single pin mode
} ds18x20_gpios[MAX_DSB];
uint8_t ds18x20_ngpio = 0; // Count of GPIO found
#endif

Comment on lines +73 to +81
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why introducing a new data structure instead of modulating the size of the existing DS18X20Data between [1] and [MAX_DSB]
That should simplify the rest of the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new structure uses only 12 bytes of RAM. DS18X20Data uses 22 bytes and increasing to 4 copies will add another 66 bytes.
And will also have to pass GPIO numbers to all low-level functions: OneWire1ReadBit(void), OneWire2ReadBit(void), OneWireWriteBit(uint8_t v), OneWireReset(void). Such changes can lead to a significant increase in the final code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was wondering : passing some parameters to function vs copying

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a test module, replaced copying with transferring the index of the gpio array.
With copying use 687300 bytes of flash.
With passing the parameter to the low-level functions 687364 bytes of flash.
Loss of 64 bytes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for trying

struct {
#ifdef W1_PARASITE_POWER
uint32_t w1_power_until = 0;
Expand Down Expand Up @@ -301,15 +315,44 @@ bool OneWireCrc8(uint8_t *addr) {
/********************************************************************************************/

void Ds18x20Init(void) {
DS18X20Data.pin = Pin(GPIO_DSB);
DS18X20Data.input_mode = Settings->flag3.ds18x20_internal_pullup ? INPUT_PULLUP : INPUT; // SetOption74 - Enable internal pullup for single DS18x20 sensor

uint64_t ids[DS18X20_MAX_SENSORS];
DS18X20Data.sensors = 0;

#ifdef DS18x20_MULTI_GPIOs
ds18x20_ngpio=0;
uint8_t pins;
for (pins = 0; pins < MAX_DSB; pins++) {
if (PinUsed(GPIO_DSB, pins)) {
ds18x20_gpios[pins].pin = Pin(GPIO_DSB, pins);

if (PinUsed(GPIO_DSB_OUT, pins)) {
ds18x20_gpios[pins].dual_mode = true;
ds18x20_gpios[pins].pin_out = Pin(GPIO_DSB_OUT, pins);
}
ds18x20_ngpio++;
}
}

for (pins = 0; pins < ds18x20_ngpio; pins++) {
DS18X20Data.pin = ds18x20_gpios[pins].pin;
DS18X20Data.dual_mode = ds18x20_gpios[pins].dual_mode;
if (ds18x20_gpios[pins].dual_mode) {
DS18X20Data.pin_out = ds18x20_gpios[pins].pin_out;
pinMode(DS18X20Data.pin_out, OUTPUT);
pinMode(DS18X20Data.pin, DS18X20Data.input_mode);
}
#else
DS18X20Data.pin = Pin(GPIO_DSB);

if (PinUsed(GPIO_DSB_OUT)) {
DS18X20Data.pin_out = Pin(GPIO_DSB_OUT);
DS18X20Data.dual_mode = true; // Dual pins mode as used by Shelly
pinMode(DS18X20Data.pin_out, OUTPUT);
pinMode(DS18X20Data.pin, DS18X20Data.input_mode);
}
#endif //DS18x20_MULTI_GPIOs

onewire_last_discrepancy = 0;
onewire_last_device_flag = false;
Expand All @@ -318,8 +361,6 @@ void Ds18x20Init(void) {
onewire_rom_id[i] = 0;
}

uint64_t ids[DS18X20_MAX_SENSORS];
DS18X20Data.sensors = 0;
while (DS18X20Data.sensors < DS18X20_MAX_SENSORS) {
if (!OneWireSearch(ds18x20_sensor[DS18X20Data.sensors].address)) {
break;
Expand All @@ -337,20 +378,35 @@ void Ds18x20Init(void) {
#ifdef DS18x20_USE_ID_ALIAS
ds18x20_sensor[DS18X20Data.sensors].alias=0;
#endif
#ifdef DS18x20_MULTI_GPIOs
ds18x20_sensor[DS18X20Data.sensors].pins_id = pins;
#endif //DS18x20_MULTI_GPIOs
DS18X20Data.sensors++;
}
}
#ifdef DS18x20_MULTI_GPIOs
}
#endif //DS18x20_MULTI_GPIOs

//#ifndef DS18x20_MULTI_GPIOs
for (uint32_t i = 0; i < DS18X20Data.sensors; i++) {
for (uint32_t j = i + 1; j < DS18X20Data.sensors; j++) {
if (ids[ds18x20_sensor[i].index] > ids[ds18x20_sensor[j].index]) { // Sort ascending
std::swap(ds18x20_sensor[i].index, ds18x20_sensor[j].index);
}
}
}
//#endif
AddLog(LOG_LEVEL_DEBUG, PSTR(D_LOG_DSB D_SENSORS_FOUND " %d"), DS18X20Data.sensors);
}

void Ds18x20Convert(void) {
#ifdef DS18x20_MULTI_GPIOs
for (uint8_t i = 0; i < ds18x20_ngpio; i++) {
DS18X20Data.pin = ds18x20_gpios[i].pin;
DS18X20Data.dual_mode = ds18x20_gpios[i].dual_mode;
DS18X20Data.pin_out = ds18x20_gpios[i].pin_out;
#endif
OneWireReset();
#ifdef W1_PARASITE_POWER
// With parasite power address one sensor at a time
Expand All @@ -362,6 +418,9 @@ void Ds18x20Convert(void) {
#endif
OneWireWrite(W1_CONVERT_TEMP); // start conversion, no parasite power on at the end
// delay(750); // 750ms should be enough for 12bit conv
#ifdef DS18x20_MULTI_GPIOs
}
#endif
}

bool Ds18x20Read(uint8_t sensor) {
Expand All @@ -370,6 +429,11 @@ bool Ds18x20Read(uint8_t sensor) {
int8_t sign = 1;

uint8_t index = ds18x20_sensor[sensor].index;
#ifdef DS18x20_MULTI_GPIOs
DS18X20Data.pin = ds18x20_gpios[ds18x20_sensor[index].pins_id].pin;
DS18X20Data.pin_out = ds18x20_gpios[ds18x20_sensor[index].pins_id].pin_out;
DS18X20Data.dual_mode = ds18x20_gpios[ds18x20_sensor[index].pins_id].dual_mode;
#endif
if (ds18x20_sensor[index].valid) { ds18x20_sensor[index].valid--; }
for (uint32_t retry = 0; retry < 3; retry++) {
OneWireReset();
Expand Down Expand Up @@ -446,15 +510,14 @@ void Ds18x20Name(uint8_t sensor) {
}
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("%s%c%s"), DS18X20Data.name, IndexSeparator(), address);
#else
uint8_t print_ind = sensor +1;
#ifdef DS18x20_USE_ID_ALIAS
if (ds18x20_sensor[sensor].alias) {
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("DS18Alias%c%d"), IndexSeparator(), ds18x20_sensor[sensor].alias);
} else {
#endif
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("%s%c%d"), DS18X20Data.name, IndexSeparator(), sensor +1);
#ifdef DS18x20_USE_ID_ALIAS
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("DS18Sens"));
print_ind = ds18x20_sensor[sensor].alias;
Comment on lines -451 to +517
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing from "DS18Alias" to "DS18Sens" is now a breaking change
I know you introduced that recently so probably not much is use yet but just raising the point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I recently added this functionality and it hasn't been released yet. DS18Sens is a more correct name. I will try to avoid such edits. Also fixed aliases output to the accepted style.

}
#endif
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("%s%c%d"), DS18X20Data.name, IndexSeparator(), print_ind);
#endif
}
}
Expand Down Expand Up @@ -580,8 +643,8 @@ void CmndDSAlias(void) {

bool Xsns05(uint8_t function) {
bool result = false;

if (PinUsed(GPIO_DSB)) {
if (PinUsed(GPIO_DSB,GPIO_ANY)) {
switch (function) {
case FUNC_INIT:
Ds18x20Init();
Expand Down
65 changes: 51 additions & 14 deletions tasmota/tasmota_xsns_sensor/xsns_05_esp32_ds18x20.ino
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
xsns_05_esp32_ds18x20.ino - DS18x20 temperature sensor support for ESP32 Tasmota

Copyright (C) 2021 Heiko Krupp and Theo Arends

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
Expand All @@ -15,8 +15,9 @@

You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

Updated by md5sum-as (https://github.com/md5sum-as)
*/

#ifdef ESP32
#ifdef USE_DS18x20
Expand Down Expand Up @@ -59,23 +60,40 @@ struct {
uint8_t valid;
#ifdef DS18x20_USE_ID_ALIAS
uint8_t alias;
#endif //DS18x20_USE_ID_ALIAS
#endif //DS18x20_USE_ID_ALIAS
#ifdef DS18x20_MULTI_GPIOs
int8_t pins_id = 0;
#endif //DS18x20_MULTI_GPIOs
} ds18x20_sensor[DS18X20_MAX_SENSORS];

#include <OneWire.h>

#ifdef DS18x20_MULTI_GPIOs
OneWire *ds18x20_gpios[MAX_DSB];
uint8_t ds18x20_ngpio = 0; // Count of GPIO found
#endif

struct {
char name[17];
uint8_t sensors = 0;
} DS18X20Data;

/********************************************************************************************/

#include <OneWire.h>

OneWire *ds = nullptr;

void Ds18x20Init(void) {
ds = new OneWire(Pin(GPIO_DSB));

#ifdef DS18x20_MULTI_GPIOs
for (uint8_t pins = 0; pins < MAX_DSB; pins++) {
if (PinUsed(GPIO_DSB, pins)) {
ds18x20_gpios[pins] = new OneWire(Pin(GPIO_DSB,pins));
ds18x20_ngpio++;
}
}
#else
ds = new OneWire(Pin(GPIO_DSB));
#endif
Ds18x20Search();
AddLog(LOG_LEVEL_DEBUG, PSTR(D_LOG_DSB D_SENSORS_FOUND " %d"), DS18X20Data.sensors);
}
Expand All @@ -84,8 +102,12 @@ void Ds18x20Search(void) {
uint8_t num_sensors=0;
uint8_t sensor = 0;

#ifdef DS18x20_MULTI_GPIOs
for (uint8_t pins=0; pins < ds18x20_ngpio; pins++) {
ds=ds18x20_gpios[pins];
#endif
ds->reset_search();
for (num_sensors = 0; num_sensors < DS18X20_MAX_SENSORS; num_sensors) {
for (num_sensors; num_sensors < DS18X20_MAX_SENSORS; num_sensors) {
if (!ds->search(ds18x20_sensor[num_sensors].address)) {
ds->reset_search();
break;
Expand All @@ -99,9 +121,16 @@ void Ds18x20Search(void) {
#ifdef DS18x20_USE_ID_ALIAS
ds18x20_sensor[num_sensors].alias=0;
#endif
#ifdef DS18x20_MULTI_GPIOs
ds18x20_sensor[num_sensors].pins_id = pins;
#endif //DS18x20_MULTI_GPIOs
num_sensors++;
}
}
#ifdef DS18x20_MULTI_GPIOs
}
#endif //DS18x20_MULTI_GPIOs

for (uint32_t i = 0; i < num_sensors; i++) {
ds18x20_sensor[i].index = i;
}
Expand All @@ -116,10 +145,17 @@ void Ds18x20Search(void) {
}

void Ds18x20Convert(void) {
#ifdef DS18x20_MULTI_GPIOs
for (uint8_t i = 0; i < ds18x20_ngpio; i++) {
ds=ds18x20_gpios[i];
#endif
ds->reset();
ds->write(W1_SKIP_ROM); // Address all Sensors on Bus
ds->write(W1_CONVERT_TEMP); // start conversion, no parasite power on at the end
// delay(750); // 750ms should be enough for 12bit conv
#ifdef DS18x20_MULTI_GPIOs
}
#endif
}

bool Ds18x20Read(uint8_t sensor, float &t) {
Expand All @@ -130,7 +166,9 @@ bool Ds18x20Read(uint8_t sensor, float &t) {

uint8_t index = ds18x20_sensor[sensor].index;
if (ds18x20_sensor[index].valid) { ds18x20_sensor[index].valid--; }

#ifdef DS18x20_MULTI_GPIOs
ds=ds18x20_gpios[ds18x20_sensor[index].pins_id];
#endif
ds->reset();
ds->select(ds18x20_sensor[index].address);
ds->write(W1_READ_SCRATCHPAD); // Read Scratchpad
Expand Down Expand Up @@ -185,15 +223,14 @@ void Ds18x20Name(uint8_t sensor) {
}
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("%s%c%s"), DS18X20Data.name, IndexSeparator(), address);
#else
uint8_t print_ind = sensor +1;
#ifdef DS18x20_USE_ID_ALIAS
if (ds18x20_sensor[sensor].alias) {
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("DS18Alias%c%d"), IndexSeparator(), ds18x20_sensor[sensor].alias);
} else {
#endif
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("%s%c%d"), DS18X20Data.name, IndexSeparator(), sensor +1);
#ifdef DS18x20_USE_ID_ALIAS
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("DS18Sens"));
print_ind = ds18x20_sensor[sensor].alias;
}
#endif
snprintf_P(DS18X20Data.name, sizeof(DS18X20Data.name), PSTR("%s%c%d"), DS18X20Data.name, IndexSeparator(), print_ind);
#endif
}
}
Expand Down Expand Up @@ -315,7 +352,7 @@ void CmndDSAlias(void) {
bool Xsns05(uint8_t function) {
bool result = false;

if (PinUsed(GPIO_DSB)) {
if (PinUsed(GPIO_DSB,GPIO_ANY)) {
switch (function) {
case FUNC_INIT:
Ds18x20Init();
Expand Down