RobTillaart / CRC

CRC library for Arduino
MIT License
81 stars 17 forks source link

Add "ifndef ... define..." to prevent already defined errors #21

Closed platenyponchet closed 2 years ago

platenyponchet commented 2 years ago

Prevent already defined erros in multiple includes.

platenyponchet commented 2 years ago

In fact, the #pragma once should fix that, but this does not work for the file CRC.h The pragma works for the other headers. I'm using vscode and platformio, building for esp8266

RobTillaart commented 2 years ago

Does the same problem appear if you build for the ESP32 or UNO? Can you post the complete error code you get? Can you post a minimal sketch that shows this problem?

I made a simple sketch and included CRC.h twice and it worked with the Arduino IDE compiling for a generic ESP8266 So I am not able to replicate your problem yet.

Strange as you state it works for the other headers but not for the CRC.h file. What do you mean by other headers? (I assume the CRC12.h etc)

RobTillaart commented 2 years ago

@platenyponchet Is this still an issue?

RobTillaart commented 2 years ago

Possible solution might be a wrapper file CRC_ALL.h

#ifndef CRC_ALL_FILES_H
#define CRC_ALL_FILES_H

#include CRC.h
#include CRC8.h
#include CRC12.h
#include CRC16.h
#include CRC32.h
#include CRC64.h

#endif 
RobTillaart commented 2 years ago

In fact, the #pragma once should fix that, but this does not work for the file CRC.h

Are there multiple CRC.h files in your environment?

RobTillaart commented 2 years ago

@platenyponchet Is this still an issue?

As there is no answer I assume it is solved.

mec-kon commented 2 years ago

I got the same error. If I put the include command not in the main.cpp but in a separate header, I get a "multiple definitions error". Im using Platformio with an ESP32-S2.

My file structure:

main.cpp:

#include <Arduino.h>
#include <Foo.h>

Foo.h:

#include <Arduino.h>
#include <CRC.h>

(code parts updated)

RobTillaart commented 2 years ago

@mec-kon The #pragma once should also handle the above case.

I will try to recreate the issue asap (can take a few days)

RobTillaart commented 2 years ago

Had a quick try (In Arduino IDE - ESP32 + UNO tested) no error.

//    FILE: CRC_include_test.ino
//  AUTHOR: Rob Tillaart
// PURPOSE: test
//     URL: 

#include "Arduino.h"
#include "CRC.h"
#include "foo.h"
#include "bar.h"

void setup() 
{
  Serial.begin(115200);
  while(!Serial);
  Serial.println(__FILE__);
}

void loop()
{
}

foo.h and bar.h both have the line #include CRC.h

Q: Can you post the exact output of the error

RobTillaart commented 2 years ago

@ivankravets Are you familiar with this problem? As I do not use PlatformIO for my project I cannot verify.

ivankravets commented 2 years ago

@RobTillaart you need to wrap your header files with Header guards.

When this issue can happen? When the user's project contains multiple header files (not 1 INO file that is classic for Arduino IDE where all code is located in one file) and some of them include #include "CRC.h" more than once. Another case could be when some library depends on CRC library and in addition, project also depends on CRC.h.

RobTillaart commented 2 years ago

@ivankravets Thanks for the quick response, I know the concept of Header guards and I use the #pragma once for this.

Advantage

Disadvantage

As I used #pragma in all my libs so it will need to be reverted in all. This will be done on request.

OK, thanks.

ivankravets commented 2 years ago

Ah, sorry. It looks like the issue only arises on the oldest GCC AVR.

RobTillaart commented 2 years ago

@mec-kon Created a develop branch with extra Header Guards Please verify it works for you.

RobTillaart commented 2 years ago

Solved in #22

mec-kon commented 2 years ago

Unfortunately, the problem is still not solved.

When compiling I get the following errors: /home/.../.platformio/packages/toolchain-xtensa-esp32s2/bin/../lib/gcc/xtensa-esp32s2-elf/8.4.0/../../../../xtensa-esp32s2-elf/bin/ld: .pio/build/esp32-s2-saola-1/src/main.cpp.o: in function reverse8(unsigned char):

/home/PROJECT_PATH/.pio/libdeps/esp32-s2-saola-1/CRC/CRC.h:26: multiple definition of reverse8(unsigned char); .pio/build/esp32-s2-saola-1/src/Foo.cpp.o:/home/PROJECT_PATH/.pio/libdeps/esp32-s2-saola-1/CRC/CRC.h:26: first defined here and so on.

The platform I'm using is "Tasmota framework-arduinoespressif32 with S3 support" (https://github.com/tasmota/platform-espressif32/releases/tag/S3_alpha)

RobTillaart commented 2 years ago

So that tasmora system is not able to use the #ifndef. #define constructs either.. Mmmm...i need to think how I can recreate the problem.

RobTillaart commented 2 years ago

I can recreate the problem in the Arduino IDE (1.8.19) with the following three files

CRC_include_test.ino

//    FILE: CRC_include_test.ino
//  AUTHOR: Rob Tillaart
// PURPOSE: test namespaces
//     URL: https://github.com/RobTillaart/CRC

#include "Arduino.h"
#include "CRC.h"
#include "foo.h"

void setup() 
{
  Serial.begin(115200);
  while(!Serial);
  Serial.println(__FILE__);

  int x = reverse8(0xAA);
  Serial.println(x, HEX);

  int y = foo(0xAA);
  Serial.println(y, HEX);
}

void loop()
{
}

// -- END OF FILE --

foo.h

  #include "Arduino.h"
  #include "CRC.h"

  int foo(int n);

foo.cpp

#include "foo.h"

int foo(int n)
{
  return reverse8(n);
}

error message

C:\Users\Rob\AppData\Local\Temp\arduino_build_833418\sketch\foo.cpp.o (symbol from plugin): In function `reverse8(unsigned char)':
(.text+0x0): multiple definition of `reverse8(unsigned char)'
C:\Users\Rob\AppData\Local\Temp\arduino_build_833418\sketch\CRC_include_test.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here

repeated for a number of function.
RobTillaart commented 2 years ago

Possible solution is using namespaces

CRC_include_test.ino

//    FILE: CRC_include_test.ino
//  AUTHOR: Rob Tillaart
// PURPOSE: test namespaces
//     URL: https://github.com/RobTillaart/CRC

#include "Arduino.h"
#include "CRC.h"
#include "foo.h"

void setup() 
{
  Serial.begin(115200);
  while(!Serial);
  Serial.println(__FILE__);

  int x = reverse8(0xAA);
  Serial.println(x, HEX);

  int y = foo::foo(0xAA);
  Serial.println(y, HEX);
}

void loop()
{
}

// -- END OF FILE --

foo.h

namespace foo
{
  #include "Arduino.h"
  #include "CRC.h"

  int foo(int n);
}

foo.cpp

#include "foo.h"

int foo::foo(int n)
{
  return reverse8(n);
}

no error messages

see also: https://www.geeksforgeeks.org/namespace-in-c/

RobTillaart commented 2 years ago

@mec-kon I tested the previous version of CRC.h (without the Header Guards) and the namespace solution just works. Apparently the problem is not in the #ifndef #define versus #pragma once.

Please verify if the use of namespaces solves your problem.

mec-kon commented 2 years ago

I have tested out your suggested program code. Unfortunately, it still does not solve the problem. Now I get a bunch of other errors such as: /home/.../.platformio/packages/toolchain-xtensa-esp32s2/xtensa-esp32s2-elf/include/c++/8.4.0/bits/std_abs.h:52:11: error: '::abs' has not been declared

Curiously, I don't get any error with other libraries, even if I don't use a namespace. (Tried PubSubClient for example https://github.com/knolleary/pubsubclient)

RobTillaart commented 2 years ago

The only difference I notice is that CRC h defines a set of C functions and no C++ class. The latter also defines a scope by its class-name.

RobTillaart commented 2 years ago

@mec-kon I did a compare with Arduino.h which is also a set of C functions. I noticed the .h only has the interface, not the implementation of the functions e.g. millis() and shiftIn() etc.

So I split the CRC.h into a CRC.h and a CRC.cpp file and tested my example again and now my test setup worked.

Can you check if this solves the issue for you?

branch prepared in - https://github.com/RobTillaart/CRC/tree/develop

RobTillaart commented 2 years ago

Merged the develop branch, if the problem continues to exist, feel free to reopen this issue.

mec-kon commented 2 years ago

@mec-kon I did a compare with Arduino.h which is also a set of C functions. I noticed the .h only has the interface, not the implementation of the functions e.g. millis() and shiftIn() etc.

So I split the CRC.h into a CRC.h and a CRC.cpp file and tested my example again and now my test setup worked.

Can you check if this solves the issue for you?

branch prepared in - https://github.com/RobTillaart/CRC/tree/develop

Yes, it solves the issue. Many thanks for the quick help.

RobTillaart commented 2 years ago

you're welcome!