Skip to content
This repository has been archived by the owner on Feb 4, 2023. It is now read-only.

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

Closed
InventoCasa opened this issue Oct 9, 2020 · 4 comments
Labels
improvement Make it better

Comments

@InventoCasa
Copy link

InventoCasa commented Oct 9, 2020

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
Copy link
Owner

@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.

@khoih-prog khoih-prog added the improvement Make it better label Oct 9, 2020
@InventoCasa
Copy link
Author

InventoCasa commented Oct 9, 2020

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

  • declaring your functions inline
  • the old approach

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

@khoih-prog
Copy link
Owner

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

HOWTO Fix Multiple Definitions Linker Error

khoih-prog added a commit that referenced this issue Oct 15, 2020
### 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`](#38) and [`Support building in PlatformIO PR`](#20). SSee [**HOWTO Fix `Multiple Definitions` Linker Error**](https://github.com/khoih-prog/ESP_WiFiManager#HOWTO-Fix-Multiple-Definitions-Linker-Error)
2. Fix bug [/close does not close the config portal](khoih-prog/ESPAsync_WiFiManager#16).
khoih-prog added a commit that referenced this issue Oct 15, 2020
### 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`](#38) and [`Support building in PlatformIO PR`](#20). Also have a look at [**HOWTO Fix Multiple Definitions Linker Error**](https://github.com/khoih-prog/ESP_WiFiManager#HOWTO-Fix-Multiple-Definitions-Linker-Error)
2. Fix bug [/close does not close the config portal](khoih-prog/ESPAsync_WiFiManager#16).
@khoih-prog
Copy link
Owner

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.

khoih-prog added a commit to khoih-prog/ESPAsync_WiFiManager that referenced this issue Oct 16, 2020
### 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`](khoih-prog/ESP_WiFiManager#38) and [`Support building in PlatformIO PR`](khoih-prog/ESP_WiFiManager#20). Also have a look at [**HOWTO Fix Multiple Definitions Linker Error**](https://github.com/khoih-prog/ESPAsync_WiFiManager#HOWTO-Fix-Multiple-Definitions-Linker-Error)
2. Fix bug [/close does not close the config portal](#16).
khoih-prog added a commit to khoih-prog/ESPAsync_WiFiManager that referenced this issue Oct 16, 2020
### 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`](khoih-prog/ESP_WiFiManager#38) and [`Support building in PlatformIO PR`](khoih-prog/ESP_WiFiManager#20). Also have a look at [**HOWTO Fix Multiple Definitions Linker Error**](https://github.com/khoih-prog/ESPAsync_WiFiManager#HOWTO-Fix-Multiple-Definitions-Linker-Error)
2. Fix bug [/close does not close the config portal](#16).
khoih-prog added a commit to khoih-prog/ESPAsync_WiFiManager that referenced this issue Oct 16, 2020
### 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`](khoih-prog/ESP_WiFiManager#38) and [`Support building in PlatformIO PR`](khoih-prog/ESP_WiFiManager#20). Also have a look at [**HOWTO Fix Multiple Definitions Linker Error**](https://github.com/khoih-prog/ESPAsync_WiFiManager#HOWTO-Fix-Multiple-Definitions-Linker-Error)
2. Fix bug [/close does not close the config portal](#16).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
improvement Make it better
Projects
None yet
Development

No branches or pull requests

2 participants