adafruit / Adafruit_nRF52_Arduino

Adafruit code for the Nordic nRF52 BLE SoC on Arduino
Other
601 stars 489 forks source link

Fix wrongful inclusion of Adafruit nRFCrypto library #738

Closed maxgerhardt closed 2 years ago

maxgerhardt commented 2 years ago

Fixes https://github.com/adafruit/Adafruit_nRF52_Arduino/issues/737

PlatformIO's default LDF search mode sees the code at

https://github.com/adafruit/Adafruit_nRF52_Arduino/blob/10ed826056f9c0b3079129a3ec37c3a90ecb0585/libraries/Bluefruit52Lib/src/BLESecurity.h#L31-L33

and always includes the crypto lib.

Fix is turning up the LDF mode (2) to one that evaluates the macro correctly and prevents the inclusion of the library when NRF_CRYPTOCELL is not defined for boards that don't have it.

I still have to test this..

ladyada commented 2 years ago

we really really do not want to do this, then we have to maintain two library files.

@ivankravets is there a way to fix this so platformio stops misparsing C files, maybe a different preparser? this issue keeps coming up where valid C code that has ifdefs doesnt work with platformio. or have some inline alert to tell platformio. thank you!

maxgerhardt commented 2 years ago

I understand that two version fields are bad -- but I still wanted to try to just have

{
  "build":
  {
    "libLDFMode": "chain+"
  }
}

so that.. maybe.. PlatformIO gets version info from library.properties and this critical build info from the library.json, hence no double version maintaining.

ladyada commented 2 years ago

yes that would be ideal... or have some extra line in the library.properties file

ivankravets commented 2 years ago

PlatformIO Core already enables "Arduino Preprocessing" for a library if it has a depends field. The problem is linked with the version of this Arduino Core that is hosted in our registry. It has patched libraries. See the contents of ~/.platformio/packages/framework-arduinoadafruitnrf52/libraries/Bluefruit52Lib.

@maxgerhardt, please move this issue to https://github.com/platformio/platform-nordicnrf52/issues

P.S: See ~/.platformio/packages/framework-arduinoadafruitnrf52/libraries/Bluefruit52Lib/extra_script.py:


# The only purpose of this extra script is to set the `NRF_CRYPTOCELL` macro for the
# nrf52840 series because the original `NRF_CRYPTOCELL` macro is set in the framework
# sources and not visible when PlatformIO LDF resolves dependencies.
if env.BoardConfig().get("build.mcu", "").startswith("nrf5284"):
    env.Append(
        CPPDEFINES=[("NRF_CRYPTOCELL", "((NRF_CRYPTOCELL_Type*) NRF_CRYPTOCELL_BASE)")],

    )