Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean Session #6

Closed
fabianbusch opened this issue Jun 16, 2019 · 8 comments
Closed

Clean Session #6

fabianbusch opened this issue Jun 16, 2019 · 8 comments
Assignees

Comments

@fabianbusch
Copy link

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
Copy link
Contributor

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);
  1. 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
Copy link

tigoe commented Sep 9, 2019

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

@fabianbusch
Copy link
Author

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
Copy link
Contributor

per1234 commented Sep 10, 2019

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):
espressif/arduino-esp32#3191

@sandeepmistry
Copy link
Contributor

@tigoe @fabianbusch thanks for the feedback!

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

@manchoz
Copy link

manchoz commented Mar 11, 2020

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

@aentinger
Copy link
Contributor

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

@aentinger aentinger self-assigned this Mar 12, 2020
@aentinger
Copy link
Contributor

Fixed by #12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants