Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow compilation in recent ARDUINO ESP32 RELEASES 2.0.1 and 2.0.2… #502

Conversation

Martin-Laclaustra
Copy link
Contributor

… by excluding AudioGeneratorMIDI
This is a temporary solution that drops MIDI support to make this library available in most recent ESP32-Arduino cores, which have a compiler bug that renders this code incompatible.

(Temporary) Fix for:
#498
#464
#440

@Martin-Laclaustra
Copy link
Contributor Author

I see that "core_version.h" is not available in all contexts (I verified it was in ESP32 and ESP8266). May be this proposal serves as inspiration for some to find a general solution. (Mine works fine locally in my computer)

@Martin-Laclaustra
Copy link
Contributor Author

I fixed the error arising from "core_version.h" not being available in all contexts.
Currently some tests fail, but the errors seem unrelated to this pull request.
I hope that this can be merged and this library be made available again for ESP32 in the most recent core releases.

Copy link
Owner

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is the GCC version, so why not just use

#if __GNUC__ == 8
// Do not build, GCC8 has a compiler bug
#else
....

That way if V2.0.3 comes out w/the same compiler it's still good.

@earlephilhower
Copy link
Owner

Oh, and thanks for the PR!

@Martin-Laclaustra
Copy link
Contributor Author

Done!

Copy link
Owner

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@shuai132
Copy link
Contributor

shuai132 commented Aug 31, 2022

Should we remove the code now? or make it configable.

#if __GNUC__ == 8
// Do not build, GCC8 has a compiler bug
#else
....

Because MIDI works on gcc8.4 patch3. and my esp8266 use gcc8 too. We can't use midi if keep it.

This is esp32:

  • framework-arduinoespressif32 @ 3.20004.220818 (2.0.4)
  • tool-esptoolpy @ 1.30300.0 (3.3.0)
  • toolchain-riscv32-esp @ 8.4.0+2021r2-patch3

And when esp8266 use 10.3, GNUC is equals 8 too!!!

 - framework-arduinoespressif8266 @ 3.30002.0 (3.0.2) 
 - tool-esptool @ 1.413.0 (4.13) 
 - tool-esptoolpy @ 1.30000.201119 (3.0.0) 
 - toolchain-xtensa @ 2.100300.210717 (10.3.0)
 ➜  .platformio packages/toolchain-xtensa/bin/xtensa-lx106-elf-gcc --version
xtensa-lx106-elf-gcc (GCC) 10.3.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

@earlephilhower

@earlephilhower
Copy link
Owner

@shuai132, I am not seeing GNUC as 8 in 10.3. Here is my output from the ESP8266 core:

echo | ~/Arduino/hardware/esp8266com/esp8266/tools/xtensa-lx106-elf/bin/xtensa-lx106-elf-gcc -dM -E - | grep GNU
#define __GNUC_PATCHLEVEL__ 0
#define __GNUC__ 10
#define __GNUC_STDC_INLINE__ 1
#define __GNUC_MINOR__ 3

I would be fine adding additional conditions to the #if. Can you run the same command line with your working GCC 8.4 so we can see what defines come out? Maybe we can change it to

#if (__GNUC__ == 8) && (__GNUC_MINOR__ < 4)
// GCC bug prevents compilation
#else
...

@shuai132
Copy link
Contributor

Sorry, 10.3 is correct, it's my IDE(clion) issue(host compiler's notice).
I'm using platformio:
toolchain path: ~/.platformio/packages/toolchain-xtensa/bin

➜  bin echo | ./xtensa-lx106-elf-gcc -dM -E - | grep GNU
#define __GNUC_PATCHLEVEL__ 0
#define __GNUC__ 10
#define __GNUC_STDC_INLINE__ 1
#define __GNUC_MINOR__ 3

BTW:
we do need like this:

#if (__GNUC__ == 8) && (__GNUC_MINOR__ < 4)

I have tested it, it works on my esp32 compiler.

@earlephilhower
Copy link
Owner

@shuai132 great. Would you like to make the PR adding the && (__GNUC_MINOR__ < 4) to the condition?

@shuai132
Copy link
Contributor

Thanks,my pleasure,
PR: #567

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants