milesburton / Arduino-Temperature-Control-Library

Arduino Temperature Library
https://www.milesburton.com/w/index.php/Dallas_Temperature_Control_Library
973 stars 487 forks source link

warning: type 'struct DallasTemperature' violates the C++ One Definition Rule [-Wodr] #240

Closed kava60 closed 1 year ago

kava60 commented 1 year ago

Hi I'm using the DallasTemperature library (Arduino Pro Mini) and am getting a bunch of warnings from the compiler relating to mismatched definitions in this library.

I've configured it to turn off both the ALARMS and NEW options:

define REQUIRESALARMS false

define REQUIRESNEW false

include

The root cause of the problem is that when the DallasTemperature.cpp is compiled it includes DallasTemperature.h and since those symbols aren't defined, the defaults are used as set in the header file (REQUIRESNEW false, REQUIRESALARMS true). When I come around and compile my code with the header, I've set REQUIRESALARMS to false so there ends up a mismatch between the class definition I'm using and what has been compiled into the DallasTemperature.o.

I'm not sure that this causes any actual problems at run time in my use I haven't seen any; but I'm obsessive about cleaning up warnings and hate seeing these roll by each time I build. There may be a problem for someone who sets REQUIRESNEW to true in their code before including DallasTemperature.h because the DallasTemperature.cpp will not have the corresponding code for the operator new compiled in...

I'm not sure of the best solution here (or I would certainly suggest one) but wanted to raise this.

Thanks for the library, Kurt V

RobTillaart commented 1 year ago

@kava60 You might try to add the following lines in the library. (Sorry, no time to test it myself)

#ifndef REQUIRESALARMS 
#define REQUIRESALARMS false
#endif 

#ifndef REQUIRESNEW 
#define REQUIRESNEW false
#endif 
kava60 commented 1 year ago

@kava60 You might try to add the following lines in the library. (Sorry, no time to test it myself)

#ifndef REQUIRESALARMS 
#define REQUIRESALARMS false
#endif 

#ifndef REQUIRESNEW 
#define REQUIRESNEW false
#endif 

Thanks - yes, that is exactly what I had done to work around the problem but I opened the issue because I thought others would run into the same problem - you shouldn't have to modify the Arduino Library source/header - especially when it is advertised that you can exclude/include functionality by using #defines in your code prior to #including the library's header. Kurt

RobTillaart commented 1 year ago

@kava60 looked into https://github.com/milesburton/Arduino-Temperature-Control-Library/blob/master/DallasTemperature.h (latest) and it seems already in the library.

kava60 commented 1 year ago

Thanks - I mis-read the README.md for the library; I thought it said to #define those symbols in your application code prior to #including the DallasTemperature.h header. It doesn't - it says (as you are pointing out) to modify the library header.

Sorry for the noise here - all a misunderstanding on my part. I'll close this thread.

Kurt