daniloc / PicoW_HomeAssistant_Starter

Everything you need to get started with your own Intranet of Things, using the high-quality, low-cost Pico W as the backbone.
MIT License
198 stars 20 forks source link

Missing MQTT_LOGIN & MQTT_PASSWORD still indicates connected to MQTT broker. #7

Open JacobChrist opened 1 year ago

JacobChrist commented 1 year ago

If MQTT_LOGIN & MQTT_PASSWORD left blank:

#define MQTT_LOGIN ""
#define MQTT_PASSWORD ""

Application still indicates that it connects to the MQTT Broker but Controls do not work.

--- More details at https://bit.ly/pio-monitor-filters
--- Quit: Ctrl+C | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H
21:39:44.381 > Connected to
21:39:44.381 > Tajin
21:39:44.382 > Connecting to MQTT
21:39:44.383 > Connected to MQTT broker

If I add the LOGIN and PASSWORD everything works fine.

daniloc commented 1 year ago

Interesting. I'll have a look at that, thanks for the report.

timmillwood commented 1 year ago

I had a similar issue, I fudged the IP address for MQTT, but it still said "Connected to MQTT broker" in the serial monitor.

Looking at the code, I would blame the upstram library, because it only returns false if it's missing a unique ID or if the connection is already initialized.

daniloc commented 1 year ago

@timmillwood thanks for digging into this, this JUST bit me again a few weeks back

Any chance I could trouble you for a link to the offending lines in arduino-home-assistant? AHA's author has gone quiet and I've been working on a fork to address my own issues with it.

timmillwood commented 1 year ago

@daniloc https://github.com/dawidchyrzynski/arduino-home-assistant/blob/main/src/HAMqtt.cpp#L89-L97

JacobChrist commented 11 months ago

If you follow the call path from this point in HAIntegration.cpp:

    if (mqtt.begin(MQTT_BROKER, MQTT_LOGIN, MQTT_PASSWORD) == true) {
        Serial.print("Connected to MQTT broker\n");
    } else {
        Serial.print("Could not connect to MQTT broker\n");
    }

calls:

bool HAMqtt::begin(const IPAddress serverIp, const char* username, const char* password)

which calls:

bool HAMqtt::begin(const char* serverHostname, const uint16_t serverPort, const char* username, const char* password)

which sets a call back function:

_mqtt->setCallback(onMessageReceived);

Attempts to connecting to the mqtt server only happens in the

void HAMqtt::connectToServer()

function which is called in the HAMqtt:loop() function.

Probably would would be better is to interpret the result from mqtt.begin not as a successful connection but a successful instantiation of the object.

So maybe change this line:

Serial.print("Could not connect to MQTT broker");

to this:

Serial.print("Could (re)instantiate MQTT object");

The (re) is because it looks like you would get a false if you tried to call mqtt.begin() twice as well.

JacobChrist commented 11 months ago

I found this note in the ArduinoHA documentation this morning that confirms intent.