stefanbode / Sonoff-Tasmota

Provide ESP8266 based itead Sonoff with Web, MQTT and OTA firmware using Arduino IDE, enhanced with I2C options
GNU General Public License v3.0
126 stars 41 forks source link

error in counterdevider in xsns_01_counter.ino #36

Closed roblad closed 6 years ago

roblad commented 6 years ago

Hi,

I discovered that COUNTERDEVIDER is not being set during compilation

// STB mode //avoid DIV 0 on unitiialized

  if (Settings.pulse_devider[i] == 0 || Settings.pulse_devider[i] == 65535 ) {
    Settings.pulse_devider[i] = COUNTERDEVIDER;
  }

// end

but Settings.pulse_devider[i] will never be set when do not have 0 or 65535 only through CMD it can be set in my opinion should be should not be set somwere else

Settings.pulse_devider[i] = COUNTERDEVIDER;

because COUNTERDEVIDER does not set value in user_config.h

stefanbode commented 6 years ago

Yes indeed this is strange code looking without context. The idea was to set it only, if there is a meaningful value inside, but the if clause seems to be wrong. Based on the branching in my configuration dump it can easily happen that on new introduced variables I get a 0 or a FF. In General the clause is ok.

No, now I r member. This is right because this is only on initialization. Does mean if there is e.g. 5 inside do not change because this is configuration setting , but if it is 0 then it is a init. value that needs to be corrected. The clause should only executed if wrong values at boot time are in the configuration. Let’s. Check where your error is. The IF seems to be ok.

roblad commented 6 years ago

Hi,

I tested and suggested code is ok in that file but in set command there is an error. Divider must be also set there, I suggest the code in sonoff.ino about line 778 (release o)
RtcSettings.pulse_counter[index -1] keeps the value multiply by Settings.pulse_devider[index -1] and huge variable is needed in my case divider 4100 unsigned long that already has ben set

now the command response and sprinf on www shows correct values, may be there is a possibility some how tu keep proper value in RtcSettings.pulse_counter[index -1] as displayed. For saved value the huge value is also stored. Values need to be extended to payload32.

else if ((CMND_COUNTER == command_code) && (index > 0) && (index <= MAX_COUNTERS)) { if ((data_len > 0) && (pin[GPIO_CNTR1 + index -1] < 99)) { //stb mod RtcSettings.pulse_counter[index -1] = payload32 Settings.pulse_devider[index -1]; Settings.pulse_counter[index -1] = payload32 Settings.pulse_devider[index -1]; //stb end } //stb mod snprintf_P(mqtt_data, sizeof(mqtt_data), S_JSON_COMMAND_INDEX_LVALUE, command, index, (RtcSettings.pulse_counter[index -1]/Settings.pulse_devider[index -1])); //end } else if ((CMND_COUNTERTYPE == command_code) && (index > 0) && (index <= MAX_COUNTERS)) { if ((payload >= 0) && (payload <= 1) && (pin[GPIO_CNTR1 + index -1] < 99)) { bitWrite(Settings.pulse_counter_type, index -1, payload &1); RtcSettings.pulse_counter[index -1] = 0; Settings.pulse_counter[index -1] = 0; } snprintf_P(mqtt_data, sizeof(mqtt_data), S_JSON_COMMAND_INDEX_NVALUE, command, index, bitRead(Settings.pulse_counter_type, index -1)); } else if (CMND_COUNTERDEBOUNCE == command_code) { if ((data_len > 0) && (payload16 < 32001)) { Settings.pulse_counter_debounce = payload16; } snprintf_P(mqtt_data, sizeof(mqtt_data), S_JSON_COMMAND_NVALUE, command, Settings.pulse_counter_debounce); } //STB mod else if ((CMND_COUNTERDEVIDER == command_code) && (index > 0) && (index <= MAX_COUNTERS)) { if (data_len > 0) { unsigned long _counter; _counter = Settings.pulse_devider[index -1]; Settings.pulse_devider[index -1] = payload16; RtcSettings.pulse_counter[index -1] = _counter * Settings.pulse_devider[index -1]; } snprintf_P(mqtt_data, sizeof(mqtt_data), S_JSON_COMMAND_INDEX_NVALUE, command, index, Settings.pulse_devider[index -1]); } //end