[MQTT] Allow client ID + longer user/pass credentials #2972

TD-er · 2020-03-29T00:44:00Z

Also moved all 'global' MQTT settings to the controller settings.

TD-er · 2020-03-29T00:44:31Z

@uzi18 Would be appreciated if you can have a look at this.

TD-er · 2020-03-29T00:48:32Z

Example of the new controller settings page.

User/pass field can be up-to 128 bytes (can be increased by setting a single define, settings compatible)
All user/pass values for all controllers must be within 1 kB (1 byte extra per field)
Added MQTT client ID template with support for system variables
Moved all global (Tools => Advanced) MQTT settings to the controller settings
Did some clean-up in the code for storing settings.

TD-er · 2020-03-29T11:42:37Z

I made a test build: ESPEasy_mega-20200328-6-PR_2972.zip

Also noticed these changes do add up to the build size (surprise!) so I will also try to add #ifdef wrappers to some code for the minimal setup.

denisfrench · 2020-03-30T10:14:10Z

Connection to Losant works with ESP_Easy_mega-20200328-6-PR_2972_normal_ESP8266_4M1M.bin using a 34 char username and 64 char password.

Unfortunately the connection is immediately dropped (a Losant quirk), because the LWT message with retain is still sent despite the checkboxes being unchecked. I suspect this is a separate issue to this PR, as the behaviour is replicated with normal releases when I extend the length of the controlleruser and controllerpassword within SecurityStruct.h .

TD-er · 2020-03-30T11:58:41Z

OK, thanks for testing.
Just to be sure about the LWT retain. Should it be sent at all, just with the retain flag off?

uzi18 · 2020-03-30T21:31:12Z

@TD-er I see huge changes - first eye on this did not reveal anything suspicious, will look more into new save/load strings routines.
Maybe we can use here ready classes for stringlist from SmingHub?

TD-er · 2020-03-30T23:06:47Z

Maybe we can use here ready classes for stringlist from SmingHub?

Do you have a link?

denisfrench · 2020-03-31T08:56:31Z

Losant will drop the connection if any message is sent with the retain flag set. It will accept a (pointless) LWT message without the retain flag and treat it like any other message.

Testing against another broker indicates that despite both "Send LWT to broker" and "Will Retain" being unchecked, a LWT message is still being sent with the retain flag set.

I am pretty sure this was also the case when I modified other recent builds to extend the length of controlleruser and controllerpassword. Older builds (before these checkboxes were available) worked fine with Losant when I made the changes and just hard-coded the LWT retain flag to 0.

denisfrench · 2020-03-31T09:12:14Z

I've discovered there might actually be a problem with the implementation of the long password though.

When I change an MQTT setting (eg enable to disable and back again), I need to re-enter the complete long password or the new connection will fail with invalid credentials (ie the password is not retained after a setting change).

Possibly this is by design, but I don't think it used to be this way.

TD-er · 2020-03-31T10:14:48Z

Possibly this is by design, but I don't think it used to be this way.

Depends on what you changed.
If it is just enable/disable the controller (and not toggling the enhanced password option) then it is for sure a bug.

denisfrench · 2020-03-31T10:30:17Z

From successful connections (cycling connected/dropped due LWT retain flag), disable/submit, then enable/submit results in connection failed with wrong credentials. Blanked-out password (5 dots) is still visable in form field.

Re-entering the password resumes the successful connection/dropped cycle.

When checking the "Use Extended Credentials", the user name + pass field for the controller can be extended up-to 128 bytes. This limit is only applicable to the text field, so it can later be extended to longer when needed. All user + pass fields for all controllers must be within the 1 kByte (including 1 extra byte per field)

TD-er · 2020-03-31T14:28:06Z

I found an issue with handling the rendering of combo boxes.
Problem was that the function signatures were such that the compiler could link the wrong one leading to crashes.

The function `executeInternalCommand` now takes roughly 3 kByte less in the binary. Expansion of the macro resulted in quite some additional binary size. Now the logic is moved to a separate function.

In an attempt to make the ESP respond faster as we have a lot of reports related to timeouts and lost connections lately.

TD-er · 2020-04-02T00:04:18Z

I've made a few other changes and also a new test build: ESPEasy_mega-20200328-15-PR_2972.zip

Please let me know if it is behaving badly. (or not :) )

denisfrench · 2020-04-02T01:57:52Z

Nope; no change with the new build unfortunately. Still:

Appears to be sending an LWT message, with the retain flag set, despite both checkboxes being unchecked.
Loses the password after disable/submit, enable/submit.

TD-er · 2020-04-03T08:22:49Z

Loses the password after disable/submit, enable/submit.

That's strange as I have tested it here.
The other one (related to LWT) is not yet addressed.

Which controller do you use?

denisfrench · 2020-04-03T09:53:05Z

C005; OpenHAB/HA MQTT.

You've got me wondering if I've got a hardware issue with these unreproducable errors.

I'll dig up a D1 mini I've got in the garage somewhere and get back to you...

TD-er · 2020-04-03T11:03:45Z

Well sometimes it can also be some corrupted settings, so doesn't have to be a hardware issue.
Meaning if it does work as expected on another node, don't throw away the 'old' one.

denisfrench · 2020-04-04T02:19:00Z

Just re-checked with a fresh-out-of-the-antistatic-bag D1 mini; this build still loses the long password for me.

In all cases I am flashing the blank image before the test image, and setting up from scratch. I've even re-downloaded the PR test release.

The long password is retained over a soft or hard reset, or power-cycle, so really this isn't that big an issue. Might trip people up though having to repopulate the password when it looks like it's filled-in.

Losant accounts are free at https://accounts.losant.com/create-account if you need an example public broker with a long password to test against.

TD-er · 2020-04-04T08:28:58Z

Just to be sure, you only toggle the enable checkbox and not the "Extended credentials" checkbox?
With the last toggled, it is correct behavior the field will be truncated to shorter lengths.

denisfrench · 2020-04-04T09:07:21Z

Yep; only touched the enabled checkbox.

TD-er · 2020-04-10T19:13:05Z

@denisfrench I tried to reproduce the issue you reported, but I was not able to.
As a matter of fact, the code for this is already in use by a friend of mine for about a week, and never heard any issue with it.

So I will merge this this evening, unless anyone has any objections about it?

denisfrench · 2020-04-11T07:22:15Z

I'm still getting the lost password after disable/enable on a custom build from the merged source, which I can't explain, but can live with. Probably related is that the long password was not successfullly set by custom.h despite #define DEFAULT_USE_EXTD_CONTROLLER_CREDENTIALS being set to true.

Should I raise a separate issue about sending the LWT message when disabled in the UI, or was this unable to be reproduced as well?

TD-er · 2020-04-11T08:24:58Z

I guess that last part of setting defaults in the Custom.h could be it.
Please make an issue for it.

TD-er mentioned this pull request Mar 29, 2020

Feature/ethernet support #2962

Closed

TD-er force-pushed the feature/MQTTclientID branch from c240eca to ab84b38 Compare March 30, 2020 13:26

TD-er added 5 commits March 31, 2020 16:11

[MQTT] Add checkbox for extended Controller Settings

d011d20

[Storage] Move SettingsType code to separate .h/.cpp

4b4604f

[MQTT] Move global MQTT parameters to controller settings

ec7a1d2

[MQTT] Fix build issue with disabled MQTT

95010a3

TD-er force-pushed the feature/MQTTclientID branch from ab84b38 to 95010a3 Compare March 31, 2020 14:11

TD-er added 5 commits March 31, 2020 23:31

[PIO] Change depicated -Wl,-T into board_build.ldscript

e77710a

[Settings] Improve SettingsType handling

2465418

Fix reporting binaryFileName in sysvars page

71469fb

[Build size] Optimize command handling macro to reduce build size

2251ebb

The function `executeInternalCommand` now takes roughly 3 kByte less in the binary. Expansion of the macro resulted in quite some additional binary size. Now the logic is moved to a separate function.

[LWIP2] Change LWIP2 from low mem to high bandwidth

ae2b23e

In an attempt to make the ESP respond faster as we have a lot of reports related to timeouts and lost connections lately.

denisfrench mentioned this pull request Apr 4, 2020

Cayenne-MQTT Support for ESPEasy #752

Closed

TD-er added 2 commits April 6, 2020 15:19

Merge branch 'mega' into feature/MQTTclientID

ec58649

Merge branch 'mega' into feature/MQTTclientID

c66cfad

TD-er mentioned this pull request Apr 9, 2020

increase max field length #2498

Closed

TD-er added 2 commits April 10, 2020 19:04

Merge branch 'mega' into feature/MQTTclientID

07c8d95

Merge branch 'mega' into feature/MQTTclientID

f13aa86

TD-er merged commit 96d4ca2 into letscontrolit:mega Apr 10, 2020

TD-er deleted the feature/MQTTclientID branch April 10, 2020 19:25

denisfrench mentioned this pull request Apr 12, 2020

LWT connect message with retain sent when both 'Send LWT' and 'Will Retain' off in UI #3006

Closed

14 tasks

TD-er mentioned this pull request Apr 13, 2020

Password is cleared in controller after saving and not changing pass #3015

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MQTT] Allow client ID + longer user/pass credentials #2972

[MQTT] Allow client ID + longer user/pass credentials #2972

TD-er commented Mar 29, 2020

TD-er commented Mar 29, 2020

TD-er commented Mar 29, 2020

TD-er commented Mar 29, 2020

denisfrench commented Mar 30, 2020 •

edited

Loading

TD-er commented Mar 30, 2020

uzi18 commented Mar 30, 2020

TD-er commented Mar 30, 2020

denisfrench commented Mar 31, 2020 •

edited

Loading

denisfrench commented Mar 31, 2020

TD-er commented Mar 31, 2020

denisfrench commented Mar 31, 2020

TD-er commented Mar 31, 2020

TD-er commented Apr 2, 2020

denisfrench commented Apr 2, 2020 •

edited

Loading

TD-er commented Apr 3, 2020

denisfrench commented Apr 3, 2020

TD-er commented Apr 3, 2020

denisfrench commented Apr 4, 2020

TD-er commented Apr 4, 2020

denisfrench commented Apr 4, 2020

TD-er commented Apr 10, 2020

denisfrench commented Apr 11, 2020 •

edited

Loading

TD-er commented Apr 11, 2020

[MQTT] Allow client ID + longer user/pass credentials #2972

[MQTT] Allow client ID + longer user/pass credentials #2972

Conversation

TD-er commented Mar 29, 2020

TD-er commented Mar 29, 2020

TD-er commented Mar 29, 2020

TD-er commented Mar 29, 2020

denisfrench commented Mar 30, 2020 • edited Loading

TD-er commented Mar 30, 2020

uzi18 commented Mar 30, 2020

TD-er commented Mar 30, 2020

denisfrench commented Mar 31, 2020 • edited Loading

denisfrench commented Mar 31, 2020

TD-er commented Mar 31, 2020

denisfrench commented Mar 31, 2020

TD-er commented Mar 31, 2020

TD-er commented Apr 2, 2020

denisfrench commented Apr 2, 2020 • edited Loading

TD-er commented Apr 3, 2020

denisfrench commented Apr 3, 2020

TD-er commented Apr 3, 2020

denisfrench commented Apr 4, 2020

TD-er commented Apr 4, 2020

denisfrench commented Apr 4, 2020

TD-er commented Apr 10, 2020

denisfrench commented Apr 11, 2020 • edited Loading

TD-er commented Apr 11, 2020

denisfrench commented Mar 30, 2020 •

edited

Loading

denisfrench commented Mar 31, 2020 •

edited

Loading

denisfrench commented Apr 2, 2020 •

edited

Loading

denisfrench commented Apr 11, 2020 •

edited

Loading