arduino-libraries / ArduinoMqttClient

ArduinoMqttClient Library for Arduino
GNU Lesser General Public License v2.1
192 stars 75 forks source link

Clean Session #6

Closed fabianbusch closed 4 years ago

fabianbusch commented 5 years ago

Hi, it would be nice to get a chance to set the flag for clean session to high or either low.

Best Regards

Fabian

sandeepmistry commented 5 years ago

Hi @fabianbusch,

Thanks for the feedback!

I can see two ways of doing this:

1) add an optional cleanSession parameter to connect(...):

virtual int connect(IPAddress ip, uint16_t port = 1883, bool cleanSession = true);
virtual int connect(const char *host, uint16_t port = 1883, bool cleanSession = true);

2) adding new API's

void cleanSession();
void noCleanSession();
// OR
void setCleanSession(bool cleanSession);

I have a slight preference towards 2) with setCleanSession(...). Do you have a preference?

@tigoe any thoughts on this?

tigoe commented 5 years ago

I think either is fine, though I also like the explicit setCleanSession().

fabianbusch commented 5 years ago

I think, to keep it backward compatible - I would prefer a separate API Call, too.

By the way - after the change in espressive interfaces for wifi ... there was a missing implementation of a virtual method. Has that been fixed, already?

per1234 commented 5 years ago

By the way - after the change in espressive interfaces for wifi ... there was a missing implementation of a virtual method. Has that been fixed, already?

If we are thinking of the same thing, that was just fixed on the ESP32 developers' end of things (though not released yet): https://github.com/espressif/arduino-esp32/pull/3191

sandeepmistry commented 5 years ago

@tigoe @fabianbusch thanks for the feedback!

@fabianbusch Please try out the changes in pull request #12, and let use know how it goes.

manchoz commented 4 years ago

@aentinger, @per1234 I think we could merge this, LGTM.

aentinger commented 4 years ago

Not sure if it doesn't affect ArduinoIoTCloud, I'd like to test it first.

aentinger commented 4 years ago

Fixed by #12.