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

MQTT will topic is set before /get suffix initialization #2106

Closed
tomas-bara opened this issue Jan 13, 2020 · 4 comments · Fixed by #2115
Closed

MQTT will topic is set before /get suffix initialization #2106

tomas-bara opened this issue Jan 13, 2020 · 4 comments · Fixed by #2115

Comments

@tomas-bara
Copy link
Contributor

Bug description
When MQTT getter suffix is defined it is ignored by MQTT last will topic. I think it is because it is initialized before getter / setter initialization on this line

_mqttApplyTopic(_mqtt_will, MQTT_TOPIC_STATUS);
just few lines before
_mqttApplySetting(_mqtt_getter, getter);

Steps to reproduce

  • Set #define MQTT_GETTER "/get" is CUSTOM.h

Expected behavior
When i want to use custom getter suffix i expect to have it everywhere.

  • Last will: espurna/XAXAXAXAXAXA/status/get 0 (Now wrong espurna/XAXAXAXAXAXA/status 0)
  • Status report: espurna/XAXAXAXAXAXA/status/get 1 (Now correct espurna/XAXAXAXAXAXA/status/get 1 because topic is initialized ad-hoc)

Device information

---8<-------

[038577] [MAIN] ESPURNA 1.14.1 (62ad7da3)
[038577] [MAIN] xose.perez@gmail.com
[038577] [MAIN] http://tinkerman.cat

[038578] [MAIN] CPU chip ID: 0xC1C60A
[038581] [MAIN] CPU frequency: 80 MHz
[038585] [MAIN] SDK version: 1.5.3(aec24ac9)
[038590] [MAIN] Core version: 2.3.0
[038594] [MAIN] Core revision: 9826c6d
[038595] [MAIN] Build time: 1578951594
[038598] 
[038601] [MAIN] Flash chip ID: 0x144051
[038603] [MAIN] Flash speed: 40000000 Hz
[038606] [MAIN] Flash mode: DOUT
[038609] 
[038610] [MAIN] Flash size (CHIP)   :  1048576 bytes /  256 sectors (   0 to  255)
[038621] [MAIN] Flash size (SDK)    :  1048576 bytes /  256 sectors (   0 to  255)
[038625] [MAIN] Reserved            :     4096 bytes /    1 sectors (   0 to    0)
[038633] [MAIN] Firmware size       :   457680 bytes /  112 sectors (   1 to  112)
[038639] [MAIN] Max OTA size        :   565248 bytes /  138 sectors ( 113 to  250)
[038646] [MAIN] EEPROM size         :     4096 bytes /    1 sectors ( 251 to  251)
[038653] [MAIN] Reserved            :    16384 bytes /    4 sectors ( 252 to  255)
[038660] 
[038662] [MAIN] EEPROM sectors: 251, 250
[038668] [MAIN] EEPROM current: 251
[038668] 
[038669] [MAIN] EEPROM:  4096 bytes initially |   305 bytes used ( 7%) |  3791 bytes free (92%)
[038679] [MAIN] Heap  : 36544 bytes initially | 13768 bytes used (37%) | 22776 bytes free (62%)
[038686] [MAIN] Stack :  4096 bytes initially |  1336 bytes used (32%) |  2760 bytes free (67%)
[038696] 
[038696] [MAIN] Boot version: 7
[038697] [MAIN] Boot mode: 1
[038703] [MAIN] Last reset reason: Power on
[038704] [MAIN] Last reset info: flag: 0
[038709] 
[038709] [MAIN] Board: ITEAD_SONOFF_T1_1CH
[038712] [MAIN] Support: BROKER BUTTON DEBUG_SERIAL DEBUG_TELNET DEBUG_WEB LED MDNS_SERVER MQTT NTP TELNET TERMINAL WEB 
[038723] [MAIN] OTA: ARDUINO ASYNCTCP WEB 
[038726] [MAIN] WebUI image: SMALL
[038729] 
[038732] [MAIN] Firmware MD5: 96b614f12461413acaae6aab1413ed7e
[038736] [MAIN] Power: 3178 mV
[038739] [MAIN] Power saving delay value: 10 ms
[038743] 

---8<-------
@mcspr
Copy link
Collaborator

mcspr commented Jan 14, 2020

Yep, order looks wrong here. _mqttApplyTopic(_mqtt_will, MQTT_TOPIC_STATUS); should be after getter string is set.

@tomas-bara
Copy link
Contributor Author

I would make an adjustment and pull request but I don't feel like guessing the correct code style or layout. I never seen usage of braces to separate blocks of code in one function and I don't do c code so often. I guess move _mqttApplyTopic(_mqtt_will, MQTT_TOPIC_STATUS); to separate block after block of getters and setters?

@mcspr
Copy link
Collaborator

mcspr commented Jan 17, 2020

Thanks! Any place would do, can be near json topic for example.

Braces have 2 reasons here:

  • automatic destruction of any object created on stack when block ends
  • avoid variable name collision between blocks

@tomas-bara tomas-bara changed the title MQTT "will" is set before /get and /set suffix initialization MQTT will topic is set before /get suffix initialization Jan 19, 2020
@tomas-bara
Copy link
Contributor Author

@mcspr thank you for explanation, I hope my pull request meats your standards :-)

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

Successfully merging a pull request may close this issue.

2 participants