arendst / Tasmota

Alternative firmware for ESP8266 and ESP32 based devices with easy configuration using webUI, OTA updates, automation using timers or rules, expandability and entirely local control over MQTT, HTTP, Serial or KNX. Full documentation at
https://tasmota.github.io/docs
GNU General Public License v3.0
22.09k stars 4.79k forks source link

If COUNTER1 gets 100Hz signal WebUpgrade fail #8928

Closed stefanbode closed 4 years ago

stefanbode commented 4 years ago

PROBLEM DESCRIPTION

A clear and concise description of what the problem is. Define COUNTER01 and attach a 100Hz trigger signal. Now there is no update possible via FileUpload or WebUpgrade. Device just reboots after a short time. Upgrade does work through the serial upgrade. The workaround is detaching signal or undefine COUNTER1

REQUESTED INFORMATION

Make sure your have performed every step and checked the applicable boxes before submitting your issue. Thank you!

- [ ] If using rules, provide the output of this command: `Backlog Rule1; Rule2; Rule3`:

Rules output here:

- [ ] Provide the output of this command: `Status 0`:

STATUS 0 output here:

- [ ] Provide the output of the Console log output when you experience your issue; if applicable:
  _(Please use_ `weblog 4` _for more debug information)_

Console output here:



### TO REPRODUCE
_Steps to reproduce the behavior:_

### EXPECTED BEHAVIOUR
_A clear and concise description of what you expected to happen._

### SCREENSHOTS
_If applicable, add screenshots to help explain your problem._

### ADDITIONAL CONTEXT
_Add any other context about the problem here._

**(Please, remember to close the issue when the problem has been addressed)**
arendst commented 4 years ago

Thx.

I will stall counter updates when performing an OTA.

arendst commented 4 years ago

Pls try with latest commit which still receives interrupts but returns immediatly.

If this doesn't work I have to detach counter interrupts during OTA.

stefanbode commented 4 years ago

Tried the latest release without changes. The upload via file is now working (2nd option). The upload through a webserver (option 1) is still not working. download starts but fails at 1...5%.

arendst commented 4 years ago

Stefan, pls try the latest commit. This time interrupts are detached from the GPIO's and re-attached once OTA is done.

stefanbode commented 4 years ago

Hi Theo. The "Upgrade by Webserver" still does not call the detach procedure. I assume the first fix was already sufficient but the function is not called. For the upgrade via file the function is called and everything is fine.

stefanbode commented 4 years ago

I cant see even the first LOG if update through Webserver

void CounterInterruptDisable(bool state) {
  AddLog_P2(LOG_LEVEL_INFO, PSTR("CNT: Detached interrupt for OTA state %d"),state);
  if (state) {   // Disable interrupts
    if (Counter.any_counter) {
      for (uint32_t i = 0; i < MAX_COUNTERS; i++) {
        if (PinUsed(GPIO_CNTR1, i)) {
          detachInterrupt(Pin(GPIO_CNTR1, i));
          AddLog_P2(LOG_LEVEL_INFO, PSTR("CNT: Detached interrupt for OTA %d"),i);
        }
      }
stefanbode commented 4 years ago

I assume it must be somewhere inside the:HandleUpgradeFirmwareStart

void HandleUpgradeFirmwareStart(void)
{
  if (!HttpCheckPriviledgedAccess()) { return; }

  char command[TOPSZ + 10];  // OtaUrl

  AddLog_P(LOG_LEVEL_DEBUG, PSTR(D_LOG_HTTP D_UPGRADE_STARTED));
  WifiConfigCounter();
  #ifdef USE_COUNTER
    CounterInterruptDisable(true);
  #endif

With this modification also the WebServer Upload now worked for me

arendst commented 4 years ago

Thx for doing the research. I had hoped the current location would be fine but as you noticed it isn't I will add it to the location you found.

Thx again.

arendst commented 4 years ago

Stefan,

a more logical location would be in file support_tasmota.ino replacing lines 961 to 980 with

  switch (state_250mS) {
  case 0:                                                 // Every x.0 second
    if (ota_state_flag && BACKLOG_EMPTY) {
      ota_state_flag--;
      if (2 == ota_state_flag) {
        RtcSettings.ota_loader = 0;  // Try requested image first
        ota_retry_counter = OTA_ATTEMPTS;
        ESPhttpUpdate.rebootOnUpdate(false);
        SettingsSave(1);  // Free flash for OTA update
      }
      if (ota_state_flag <= 0) {
#ifdef USE_COUNTER
        CounterInterruptDisable(true);
#endif
#ifdef USE_WEBSERVER
        if (Settings.webserver) StopWebserver();
#endif  // USE_WEBSERVER
#ifdef USE_ARILUX_RF
        AriluxRfDisable();  // Prevent restart exception on Arilux Interrupt routine
#endif  // USE_ARILUX_RF

So inserting the CounterInterruptDisable(true) at lines 972 to 974.

Could you pls test if that is OK too (so without the change in HandleUpgradeFirmwareStart())?

arendst commented 4 years ago

In the meantime the latest commit contains this change too.

stefanbode commented 4 years ago

Yes. I'm also fine with this change. Now it works and the update went through

stefanbode commented 4 years ago

During long term testing, I found a second problem with the COUNTER. Try to do the same trick with the CounterInterruptDisable and find out, that the CounterInterruptDisable(false) does NOT recreate the working status after it has disabled the Interrupt. The Counter is not available anymore. For the firmware update this is no deal because there is a reboot. But now It look like the CFG Save also requires a temporary pause of the counter. Here the CounterInterruptDisable(false) does not recover to a valid state anymore. I have on a ~72h period suddenly a reboot and then the CFG is destroyed. AP1 and AP2 for example report as values "255" The error is really hard to produce....

Only a reset 1 and a restore of the config helps to get the device back working. It also does not start in AP mode....

stefanbode commented 4 years ago

Maybe I just see a side effect. Closing it until more info available. Should work

trlafleur commented 1 year ago

I'm using a version of the base counter code in my FlowControler code and have noticed when the flow is large (lots of pulses), I'm seeing issues with the OTA update, and must wait till the flow is off or low to do an update...

This got me to this issue...

If this is a real problem, there may other devices that may have similar issues like I'm having, I might suggest that instead of doing an explicit call to CounterInterruptDisable(), you might make this an interface function command to the driver... like what is done with FUNC_EVERY_SECOND, etc...

This would make it generic and allow any driver the ability to enable/disable interrupt if needed by Tasmota...

arendst commented 1 year ago

I've thought about that too but as you are the first to detect it I just added some local solution to drivers I use in my sub-set of drivers.

I'll add it to my list and see if this can be resolved soon.

Thx.

arendst commented 1 year ago

On the other hand, I've checked in the OTA code and see that the counterInterrupts are disabled before the OTA en re-enabled after the OTA (Except ArduinoOTA which doesn't).

So what is your specific case where it fails?

trlafleur commented 1 year ago

My code does not include the counter interrupt code, use in the counter app, as my app is independent of the counter app.

On Sat, Jan 7, 2023 at 5:49 AM Theo Arends @.***> wrote:

On the other hand, I've checked in the OTA code and see that the counterInterrupts are disabled before the OTA en re-enabled after the OTA (Except ArduinoOTA which doesn't).

So what is your specific case where it fails?

— Reply to this email directly, view it on GitHub https://github.com/arendst/Tasmota/issues/8928#issuecomment-1374488145, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABC4EK576MZAUARTJ54T7SLWRFX77ANCNFSM4O6AK55A . You are receiving this because you commented.Message ID: @.***>

--

~~ /) ~~~~ /) ~~ _/) ~~ _/) ~~

Tom Lafleur

stefanbode commented 1 year ago

I would suggest not to duplicate the counter part and just enhance the missing. This would simplify and also help to keep the code small. If you can point to your function at GitHub we may help. I would go with Theo not to generate a general hook because interrupts are a bit complicated in Tasmota anyhow.

trlafleur commented 1 year ago

The way Tasmota disables interrupt to the counter driver during OTA, is brute force... (simple), if we are going to do OTA, tell the driver to disable the interrupt, then it sends an enable when finish...

So in my opinion this should be available to all driver that uses interrupt as a general hook...

just my two bits...