gysmo38 / mitsubishi2MQTT

Mitsubishi to MQTT with ESP8266 module
GNU Lesser General Public License v2.1
393 stars 141 forks source link

Loading control page without MQTT configured causes board to reset #58

Closed wabarkley closed 3 years ago

wabarkley commented 4 years ago

As also noted by @andrewl3wis in the gitter, when I first flashed my boards I attempted to load the control page and the board would reset - sending a wdt error through the serial connection.

Once I filled out the MQTT settings, the control page started working correctly. I have six boards total and all are working perfectly with MQTT enabled, I verified the board reset issue on the last one I installed before configuring MQTT. Once configured, the control page loads fine.

gysmo38 commented 4 years ago

ok thank. I will check if it possible to add a check before load the page

wabarkley commented 4 years ago

Thanks. Ideally, the control page would load and operate correctly even if mqtt wasn't configured. This repo really is super easy to use and fairly robust already. For folks that aren't necessarily interested in mqtt, this would be great to just control the heat pump.

rscoleNZ commented 3 years ago

I wish I had seen this... As a complete beginner I assumed either my soldering was wrong tx/rx won't way round, or I was about to reflash my board until I checked here. But I agree, having it work without MQTT would be ideal. Thanks for everything you've done to date.

smichtch commented 3 years ago

I ran into this as well and was convinced that I botched the crimp terminals yet again.

I think what's happening is handleControl() calls change_states(settings) which triggers hpSettingsChanged() and/or hpStatusChanged() callbacks and neither of those test whether mqtt_client is initialized before trying to publish and that must be causing the crash/reset? Also handleControl() should probably skip calling change_states(settings) on GET requests.

This is just speculation after skimming though the code, haven't had a chance to try to fix the code yet.

wabarkley commented 3 years ago

Do you think it would be more efficient to initialize the mqtt client but keep it disconnected in some way, or to wrap all the various publish commands with a mqtt_client initialization check?

wabarkley commented 3 years ago

Digging in further, I'm thinking about just setting the setSettingsChangedCallback and setStatusChangedCallback callbacks within initMQTT since I know both hp and mqtt_client will have been initialized at that point. (I'm not sure if that's bad practice, though.)

smichtch commented 3 years ago

Ok my hypothesis about hpSettingsChanged()/hpStatusChanged() was wrong. It's actually crashing in handleControl() when accessing the heatpumpSettings struct:

  if (strcmp(settings.power, "ON") == 0) {
    controlPage.replace("_POWER_ON_", "selected");
  }
  ...

Looks like the default heatpumpSettings is corrupt when loop() isn't polling the heat pump - i.e. when MQTT is not configured. The control page seems happy after changing loop() from:

  if (!captive and mqtt_config) {
    // Sync HVAC UNIT
    ...
    //MQTT failed retry to connect
    ...
  }
  else ...

to

  if (!captive) {
    // Sync HVAC UNIT
    ...
    if (mqtt_config) {
      //MQTT failed retry to connect
      ...
    }
  }
  else ...