search

khoih-prog / ESP_WiFiManager

This is an ESP32 / ESP8266 WiFi Connection Manager with fallback web configuration portal. Use this library for configuring ESP32 (including ESP32-S2 and ESP32-C3), ESP8266 modules' WiFi, etc. Credentials at runtime. You can also specify static DNS servers, personalized HostName, fixed or random AP WiFi channel. With examples supporting ArduinoJson
MIT License
371 stars 97 forks source link

Change Implementation to seperate *.h and *.cpp file instead of *.h and *-Impl.h #38

Closed InventoCasa closed 3 years ago

InventoCasa commented 3 years ago

I had a lot of problems integrating your library into my code, mainly because of the strange way of using an ESP_WiFiManager-Impl.h file instead of a .cpp file.

I got the same errors as described in this pull request --> a lot of multiple definition errors. I suggest that you change that, since most people are not used to this.

khoih-prog commented 3 years ago

@Invento3D

Thanks for using the library and your opinion.

I won't change as suggested as I don't like to loose the benefit of the current way. Many people are also able to use the library as is. The limited number of people having your kind of problem, by using complicated code, with the knowledge, should know the simple way to modify several lines of the library.

Anyway, in the next release, I'll

  1. Add the standard directory src_std beside current src source to facilitate the old-style cpp usage
  2. HOWTO Note to clarify the new addition in case of getting multiple definition errors.
InventoCasa commented 3 years ago

Sorry, but your target users are not well-experienced C++ developers, but most likely mainly people out of the maker area which will not have as much C++ experience as you may think.

Apart from that, only because header-only files have become quite popular in the C++ community does not mean that they are suited for every use case. Your header-only file basically consists out of non-template function definitions. If now the header file is included in more than one translation unit, the function would be defined multiple times.

Please consider either

I'm also not sure why you used so many preprocessor directives instead of just using constants.

khoih-prog commented 3 years ago

I'll update later with the simple solution similar to this

HOWTO Fix Multiple Definitions Linker Error

khoih-prog commented 3 years ago

See ESP_WiFiManager v1.2.0 and Contributions-and-Thanks

Releases v1.2.0

  1. Restore cpp code besides Impl.h code to use in case of multiple definition linker error. See Change Implementation to seperate *.h and *.cpp file instead of *.h and *-Impl.h and Support building in PlatformIO PR. Also have a look at HOWTO Fix Multiple Definitions Linker Error
  2. Fix bug /close does not close the config portal.