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

IRMQTTServer: add MQTT_SERVER_AUTODETECT_ENABLE #1769

Merged
merged 2 commits into from
Mar 7, 2022

Conversation

helen-fornazier
Copy link
Contributor

@helen-fornazier helen-fornazier commented Mar 4, 2022

The MQTT broker can change address if its IP is not static.

Add MQTT_SERVER_AUTODETECT_ENABLE option which detects the broker that
advertises the service _mqtt._tcp

If the broker changes address it won't be a problem. If the broker
doesn't advertise the service, then use the pre-configured IP as before.

Example testing on Debian:

$ apt install mosquitto avahi-daemon

$ cat <<EOT >> /etc/mosquitto/conf.d/my_broker.conf
listener 1883 0.0.0.0
allow_anonymous true
EOT

$ cat <<EOT >> /etc/avahi/services/mqtt.service
<?xml version="1.0" standalone='no'?><!--*-nxml-*-->
<!DOCTYPE service-group SYSTEM "avahi-service.dtd">
<service-group>
  <name replace-wildcards="yes">%h</name>
  <service protocol="ipv4">
    <type>_mqtt._tcp</type>
    <port>1883</port>
  </service>
</service-group>
EOT

$ systemctl start mosquitto
$ systemctl start avahi-daemon

Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off, thanks for this. It looks like a great addition to the example/program!

Can I get you to please bump the version number in IRMQTTServer.h to 1.7.0 please as a result of your improvement?
e.g.

#define _MY_VERSION_ "v1.7.0"

and there are a few automatic linter issue that need to be addressed as well before we can merge.
e.g.

examples/IRMQTTServer/IRMQTTServer.ino:2001:  At least two spaces is best between code and comments  [whitespace/comments] [2]
examples/IRMQTTServer/IRMQTTServer.ino:2035:  At least two spaces is best between code and comments  [whitespace/comments] [2]
examples/IRMQTTServer/IRMQTTServer.ino:2234:  At least two spaces is best between code and comments  [whitespace/comments] [2]
examples/IRMQTTServer/IRMQTTServer.ino:2350:  At least two spaces is best between code and comments  [whitespace/comments] [2]

I assume you've tested this with flag enabled and there being no zeroconf/avahi _mqtt._tcp service advertised. i.e. What happens?

examples/IRMQTTServer/IRMQTTServer.h Outdated Show resolved Hide resolved
examples/IRMQTTServer/IRMQTTServer.ino Outdated Show resolved Hide resolved
examples/IRMQTTServer/IRMQTTServer.ino Outdated Show resolved Hide resolved
examples/IRMQTTServer/IRMQTTServer.ino Outdated Show resolved Hide resolved
@crankyoldgit
Copy link
Owner

crankyoldgit commented Mar 5, 2022

Putting that new code/define etc behind the MQTT_ENABLE flag etc stops the code from breaking when MQTT is disabled.
e.g. When MQTT_ENABLE is set to false your code currently has compile errors of:

/home/runner/work/IRremoteESP8266/IRremoteESP8266/examples/IRMQTTServer/IRMQTTServer.ino: In function 'void mqtt_detect_server()':
/home/runner/work/IRremoteESP8266/IRremoteESP8266/examples/IRMQTTServer/IRMQTTServer.ino:1995:11: error: 'MqttServer' was not declared in this scope
 1995 |   strncpy(MqttServer, MDNS.IP(0).toString().c_str(), kHostnameLength);
      |           ^~~~~~~~~~
/home/runner/work/IRremoteESP8266/IRremoteESP8266/examples/IRMQTTServer/IRMQTTServer.ino:1996:11: error: 'MqttPort' was not declared in this scope; did you mean 'kHttpPort'?
 1996 |   strncpy(MqttPort, String(MDNS.port(0)).c_str(), kPortLength);
      |           ^~~~~~~~
      |           kHttpPort
*** [.pio/build/d1_mini_no_mqtt/src/IRMQTTServer.ino.cpp.o] Error 1
========================== [FAILED] Took 8.28 seconds ==========================

You can verify this by building the following target environments:

[env:d1_mini_no_mqtt]
board = d1_mini
build_flags =
${env.build_flags}
-DMQTT_ENABLE=false
lib_deps = ${common_esp8266.lib_deps_external}

or
; This is just to help enable swapping IRMQTTServer via OTA on platforms with
; limited flash space. It doesn't do *ANY* IR stuff. It has almost everything
; turned off except OTA over http.
; Produces a ".bin" file of ~380k.
[env:esp8266_1m_OTA_minimal]
board = esp01_1m
board_build.ldscript = eagle.flash.1m64.ld
build_flags =
${env.build_flags}
-DMQTT_ENABLE=false
-D_IR_ENABLE_DEFAULT_=false
-DEXAMPLES_ENABLE=false
-DMDNS_ENABLE=false
-DUSE_DECODED_AC_SETTINGS=false
lib_deps = ${common_esp8266.lib_deps_external}

@helen-fornazier
Copy link
Contributor Author

First off, thanks for this. It looks like a great addition to the example/program!

My pleasure, thanks for this example/program, it is really useful.
And thank you for the review!

Can I get you to please bump the version number in IRMQTTServer.h to 1.7.0 please as a result of your improvement? e.g.

#define _MY_VERSION_ "v1.7.0"

Sure

and there are a few automatic linter issue that need to be addressed as well before we can merge. e.g.

Which command do you use to run linter? (So I can run in my local machine too).

examples/IRMQTTServer/IRMQTTServer.ino:2001:  At least two spaces is best between code and comments  [whitespace/comments] [2]
examples/IRMQTTServer/IRMQTTServer.ino:2035:  At least two spaces is best between code and comments  [whitespace/comments] [2]
examples/IRMQTTServer/IRMQTTServer.ino:2234:  At least two spaces is best between code and comments  [whitespace/comments] [2]
examples/IRMQTTServer/IRMQTTServer.ino:2350:  At least two spaces is best between code and comments  [whitespace/comments] [2]

I assume you've tested this with flag enabled and there being no zeroconf/avahi _mqtt._tcp service advertised. i.e. What happens?

If no server is found, then it doesn't modify the variables MqttServer and MqttPort, so it uses wherever is pre-configured, or from first boot wifi config, or from loadConfigFile(), i.e. it behaves like before this patch.

If in one boot the mqtt server is found, then it modifies MqttServer and MqttPort, and they get saved if saveConfig() is called (i.e. if user modifies something in the web config page). So in the next boot, if it doesn't find the server through mDNS, it will reuse the address and port from previous boot.

The MQTT broker can change address if its IP is not static.

Add MQTT_SERVER_AUTODETECT_ENABLE option which detects the broker that
advertises the service _mqtt._tcp

If the broker changes address it won't be a problem. If the broker
doesn't advertise the service, then use the pre-configured IP as before.

Example testing on Debian:

$ apt install mosquitto avahi-daemon

$ cat <<EOT >> /etc/mosquitto/conf.d/my_broker.conf
listener 1883 0.0.0.0
allow_anonymous true
EOT

$ cat <<EOT >> /etc/avahi/services/mqtt.service
<?xml version="1.0" standalone='no'?><!--*-nxml-*-->
<!DOCTYPE service-group SYSTEM "avahi-service.dtd">
<service-group>
  <name replace-wildcards="yes">%h</name>
  <service protocol="ipv4">
    <type>_mqtt._tcp</type>
    <port>1883</port>
  </service>
</service-group>
EOT

$ systemctl start mosquitto
$ systemctl start avahi-daemon
@crankyoldgit
Copy link
Owner

Which command do you use to run linter? (So I can run in my local machine too).

cpplint --extensions=c,cc,cpp,ino --headers=h,hpp {src,src/locale,test,tools}/*.{h,c,cc,cpp,hpp,ino} examples/*/*.{h,c,cc,cpp,hpp,ino}

You will probably need to install cpplint via:

pip install cpplint

Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates. Approved.

@crankyoldgit crankyoldgit merged commit c053aa5 into crankyoldgit:master Mar 7, 2022
crankyoldgit added a commit that referenced this pull request Mar 14, 2022
_v2.8.2 (20220314)_

**[Bug Fixes]**
- ESP32-C3: Fix reboot/crashes on ESP32-C3s when receiving. (#1768 #1751)

**[Features]**
- HITACHI_AC296: Add `IRac` class support & tests. (#1776 #1758 #1757)
- Support for Hitachi RAS-70YHA3 (remote RAR-3U3) (#1758 #1757)
- LG: Add Swing Toggle support for Model `LG6711A20083V` (#1771 #1770)
- IRMQTTServer: add `MQTT_SERVER_AUTODETECT_ENABLE` via mqtt mDNS (#1769)
- Experimental basic support for Kelon 168 bit / 21 byte protocol. (#1747 #1745 #1744)
- MitsubishiAC: Tweak repeat gap timing. (#1760 #1759)
- Gree YAP0F8 (Detected as Kelvinator) vertical position set support (#1756)
- Make KELON (48 bit) protocol decoding stricter. (#1746 #1744)
- IRMQTTServer V1.6.1 (#1740 #1739 #1729)
- HITACHI_AC264: Add minimal detailed support. (#1735 #1729)
- LG2: Improve Light toggle msg handling. (#1738 #1737)
- MIDEA: Add support for Quiet, Clean & Freeze Protect controls. (#1734 #1733)
- Add basic support for HITACHI_AC264 264bit protocol. (#1730 #1729)
- ESP32-C3: Work around for some C3 specific compiler issues again. (#1732 #1695)

**[Misc]**
- MIDEA: Update supported devices (#1774 #1773 #1716)
- Update devices supported by ELECTRA_AC (#1766 #1765)
- Improve documentation for `encodePioneer()` (#1761 #1749)
- Update (un)supported DAIKIN128 devices. (#1752)
- Refactor `decodeCOOLIX()` code & add another test case. (#1750 #1748)
- Simplify code based on state_t being initialised by default. (#1736 #1699)
- Add comments to help Teknopoint users. (#1731 #1728)
- Fix library version string calculation. (#1727 #1725)
- Confirm we can reproduce `TurnOnFujitsuAC.ino` via IRac/IRMQTTServer. (#1726 #1701)
@crankyoldgit crankyoldgit mentioned this pull request Mar 14, 2022
crankyoldgit added a commit that referenced this pull request Mar 15, 2022
##_v2.8.2 (20220314)_

**[Bug Fixes]**
- ESP32-C3: Fix reboot/crashes on ESP32-C3s when receiving. (#1768 #1751)

**[Features]**
- HITACHI_AC296: Add `IRac` class support & tests. (#1776 #1758 #1757)
- Support for Hitachi RAS-70YHA3 (remote RAR-3U3) (#1758 #1757)
- LG: Add Swing Toggle support for Model `LG6711A20083V` (#1771 #1770)
- IRMQTTServer: add `MQTT_SERVER_AUTODETECT_ENABLE` via mqtt mDNS (#1769)
- Experimental basic support for Kelon 168 bit / 21 byte protocol. (#1747 #1745 #1744)
- MitsubishiAC: Tweak repeat gap timing. (#1760 #1759)
- Gree YAP0F8 (Detected as Kelvinator) vertical position set support (#1756)
- Make KELON (48 bit) protocol decoding stricter. (#1746 #1744)
- IRMQTTServer V1.6.1 (#1740 #1739 #1729)
- HITACHI_AC264: Add minimal detailed support. (#1735 #1729)
- LG2: Improve Light toggle msg handling. (#1738 #1737)
- MIDEA: Add support for Quiet, Clean & Freeze Protect controls. (#1734 #1733)
- Add basic support for HITACHI_AC264 264bit protocol. (#1730 #1729)
- ESP32-C3: Work around for some C3 specific compiler issues again. (#1732 #1695)

**[Misc]**
- MIDEA: Update supported devices (#1774 #1773 #1716)
- Update devices supported by ELECTRA_AC (#1766 #1765)
- Improve documentation for `encodePioneer()` (#1761 #1749)
- Update (un)supported DAIKIN128 devices. (#1752)
- Refactor `decodeCOOLIX()` code & add another test case. (#1750 #1748)
- Simplify code based on state_t being initialised by default. (#1736 #1699)
- Add comments to help Teknopoint users. (#1731 #1728)
- Fix library version string calculation. (#1727 #1725)
- Confirm we can reproduce `TurnOnFujitsuAC.ino` via IRac/IRMQTTServer. (#1726 #1701)
@crankyoldgit
Copy link
Owner

FYI, the changes mentioned above have now been included in the new v2.8.2 release of the library.

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

Successfully merging this pull request may close these issues.

2 participants