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

Linker error #4

Closed
bmcdonnell opened this issue May 8, 2021 · 4 comments
Closed

Linker error #4

bmcdonnell opened this issue May 8, 2021 · 4 comments
Labels
wontfix This will not be worked on

Comments

@bmcdonnell
Copy link

Thanks for your work on this.

What is the purpose of the header only implementation of this library? Since it can produce the known linker error issue, and its contents are nearly identical to the src_cpp version, I think this library would be improved by having only the src_cpp version.

Describe the bug

Linker error

multiple definition of `Timezone::display_STD_Rule()

Steps to Reproduce

Build.

Expected behavior

Clean build.

Actual behavior

Linker error.

Debug and AT-command log (if applicable)

N/A.

Screenshots

N/A.

Information

Arduino IDE version: 1.8.13
OS: Windows 10 version 1909 (OS Build 18363.1440)
Board: Adafruit Metro M4 AirLift
Board lib: Adafruit SAMD Boards library v1.6.7
  • ESP8266,ESP32 or STM32 Core Version (e.g. ESP8266 core v2.7.4, ESP32 v1.0.5 or STM32 v1.9.0)

I'm not sure if this applies. Where would I find it?


### Additional context

N/A
@khoih-prog
Copy link
Owner

Thanks for using the library.

What is the purpose of the header only implementation of this library?

But you need to do some research why header-only implementation is better. I don't like to make life easy without real benefit.

I'm not sure if this applies. Where would I find it?

Just examples for cores as this library covers too many kind of boards/cores. You need only the currently used core. such as Adafruit SAMD Boards library v1.6.7

Good Luck,

@khoih-prog khoih-prog added the wontfix This will not be worked on label May 9, 2021
@bmcdonnell
Copy link
Author

But you need to do some research why header-only implementation is better.

The "pros" such as discussed on this SO post seem like premature optimization to me. Unless you've profiled and found the need?

I don't like to make life easy without real benefit.

IMO, preventing a linker error for your users would be a real benefit. It takes time for us to find the source for the error, then your explanation, then to work around it as you've described. It complicates upgrading for us, too, since we have to modify the library each time. (Within the Arduino IDE, at least.)

Having to modify the library is unconventional and unexpected, especially since it's listed in the Arduino Library Manager.

What is the "real benefit" of having the header-only implementation in this instance?

@bmcdonnell
Copy link
Author

@khoih-prog how do you prevent this linker error from the header-only version when using this library in your own applications?

@khoih-prog
Copy link
Owner

You can have a look at a simple solution at multiple definition of `websockets2_generic::WebsocketsServer::~WebsocketsServer()' in websocket2_generic library #3

Other simpler ways to do are

  • to rename your cpp files to hpp files.
  • convert aaa.cpp files to aaa-Impl.h files

I suggest you spend time to test all the options yourself, then find out why the h-only is better or worse, in which way.

Research by Googling is good, but don't trust everything you read, especially something 10-15 years old. It's always better you verify what you read before using.

I'm sorry I won't answer this topic anymore.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants