Skip to content

ArduinoOTA undefined reference (MDNS) when compiling with -DNO_GLOBAL_INSTANCES #4643

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

Closed
everslick opened this issue Apr 14, 2018 · 9 comments

Comments

@everslick
Copy link
Contributor

The ArduinoOTA library has an hidden dependency on mDNS. When we have NO_GLOBAL_INSTANCES defined there will be no global object with the name MDNS and the compile fails.

IMHO the two libraries should work independently. I suspect just removing the MDNS call from ArduinoOTA will be met with some resistance, though.

I don't know how this could be easily fixed, besides removing the

#if !defined(NO_GLOBAL_INSTANCES) && !defined(NO_GLOBAL_MDNS)

guard in ESP8266mDNS.h, which is a bit ironic, because I was the one proposing it in the first place. And for the sake of RAM usage, I still think it is a good thing to be be able to start services on demand, instead of creating a bunch of global objects regardless of if they are needed.

On ESP32 the MDNS object is also global, but at least there is a MDNS.end() method to shut the server and free all allocated memory.

Any opinions on that?

@devyte
Copy link
Collaborator

devyte commented Dec 14, 2018

The new MDNS has a close() method. Also, I think the lines of code that make use of MDNS could be conditionally compiled with the same macros.

@everslick
Copy link
Contributor Author

everslick commented Dec 16, 2018

Thx! I have just checked out the new mDNS implementation and have two minor issues with it.

  • First, shouldn't MDNS.close() be rather called MDNS.end()? On ESP32 we have this method since ever and people who have ESP32/ESP8266 compatible firmwares need to ifdef it.

  • Second, setInstanceName() used to accept a const String &, but the new API requires str.c_str(). When functions are able to consume const String & parameters they happily accept const char * implicitly.

@everslick
Copy link
Contributor Author

Hmmm. The new mDNS implementation does not work at all here. Switching from esp8266::MDNSImplementation::MDNSResponder MDNS; back to Legacy_MDNSResponder::MDNSResponder MDNS; brings back name resolution for me.

@earlephilhower
Copy link
Collaborator

@everslick, if you're still stuck with the legacy MDNS responder, I think it would be very helpful to open a bug with the new issue so it can be tracked and fixed. We're looking to deprecate the legacy responder and fixes are probably not going to be prioritized very high.

@earlephilhower earlephilhower added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jan 31, 2019
@everslick
Copy link
Contributor Author

I will check, if I have still this problem with latest master tomorrow and report back!

@LaborEtArs
Copy link
Contributor

Please make sure, that you've got a call to MDNS::update() (or at least OTA::update()) in the loop.

@devyte
Copy link
Collaborator

devyte commented Feb 3, 2019

I think this has to do with the global MDNS instance being manipulated from ArduinoOTA. Those lines of code would need to be conditionally compiled to avoid this error.

@everslick
Copy link
Contributor Author

I think we are mixen two different issues here. The original issue was the dependency of ArduinoOTA on MDNS, which breaks the build when GLOBAL_INSTANCES is disabled. which can be fixed by using the same #ifdefs for those lines as already pointed out by @devyte.

The other issue is, that the new mDNS implementation did not work for me, when I tried it. But I have to admit, that I did not add MDNS.update(), because the old one didn't need it, and I somehow missed it.

Anyway, I will retest the new mDNS ASAP, and report back.

@everslick
Copy link
Contributor Author

Propose PR: #5768

devyte pushed a commit that referenced this issue Feb 16, 2019
Fixes #4643 

This is especially important when compiling for host mode, because ArduinoOTA is pulled in unconditionally by the Makefile in tests/host. Thus it breaks the build as soon as NO_GLOBAL_INSTANCES or NO_GLOBAL_MDNS is defined.
@devyte devyte added type: bug component: libraries and removed waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels Feb 16, 2019
@devyte devyte added this to the 2.6.0 milestone Feb 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants