sparkfun / SparkFun_SCD4x_Arduino_Library

An Arduino library for the Sensirion SCD4x (SCD40 and SCD41) family of CO2 sensors
Other
12 stars 8 forks source link

[Feature request] Enable use of the library without debug log included #3

Closed tonhuisman closed 2 years ago

tonhuisman commented 2 years ago

Working on ESPEasy, a plugin based IoT solution for ESP8266 and ESP32, we have been struggling with keeping sketch size as small as possible. Using this library to connect the SCD40/SCD41 environmental sensors does add quite some .bin size to our project, and parts of that will never be used: the debug log. There are quite a few, and rather long, flash-strings used that I'd like to exclude from our builds.

To be able to reduce sketch size, ca. 2.5 kB, I'm introducing a compiler define LIBRARIES_NO_LOG=1 on our side to exclude these unused logs from used libraries, like this:

//Enable/disable including debug log (to allow saving some space)
#ifndef SCD4x_ENABLE_DEBUGLOG
  #if defined(LIBRARIES_NO_LOG) && LIBRARIES_NO_LOG
    #define SCD4x_ENABLE_DEBUGLOG 0 // OFF/disabled/excluded on demand
  #else
    #define SCD4x_ENABLE_DEBUGLOG 1 // ON/enabled/included by default
  #endif
#endif

And in the library code I use (for example):

    #if SCD4x_ENABLE_DEBUGLOG
    if (_printDebug == true)
    {
      _debugPort->println(F("SCD4x::setSensorAltitude: periodic measurements are running. Aborting"));
    }
    #endif // if SCD4x_ENABLE_DEBUGLOG
    return (false);
  }

This will by default leave all debug code in and logging usable, only to be inhibited when LIBRARIES_NO_LOG is set to 1. (Using 0/1 as that's easier readable than false/true.)

I'll create a PR to add this to the library, and kindly request to merge and release that, so we can use this original work, instead of a copy/adjusted version in our repository.

PaulZC commented 2 years ago

Hi Ton (@tonhuisman ),

Sure - that's OK. Please send us a Pull Request and we will be happy to merge it.

We already do this on some other libraries, e.g.:

https://github.com/sparkfun/SparkFun_u-blox_GNSS_Arduino_Library/blob/20e265db38e9ae5f131a097fb75ecc62a785d7c1/src/SparkFun_u-blox_GNSS_Arduino_Library.h#L59-L60

https://github.com/sparkfun/SparkFun_u-blox_GNSS_Arduino_Library/blob/20e265db38e9ae5f131a097fb75ecc62a785d7c1/src/SparkFun_u-blox_GNSS_Arduino_Library.cpp#L102-L107

Best wishes, Paul

tonhuisman commented 2 years ago

We already do this on some other libraries

Great!

NB: I prefer to use explicitly defined compiler flags (0/1) to be able to fully control inclusion or exclusion using such flag. checking using #ifndef is often somewhat ambiguous.

tonhuisman commented 2 years ago

@PaulZC Sorry, I forgot to mention that we're building using PlatformIO, so if you can also update the library in their registry, that would be super 💯

PaulZC commented 2 years ago

@tonhuisman Sorry - you need to poke PlatformIO and ask them to update. We (SparkFun) have no control over PlatformIO content...

tonhuisman commented 2 years ago

@tonhuisman Sorry - you need to poke PlatformIO and ask them to update. We (SparkFun) have no control over PlatformIO content...

Aha, I'm not that familiar with their procedures and policies, guess it's just a waiting game, then.

Thanks for the heads up.

tonhuisman commented 2 years ago

All is well now, PlatformIO registry has been updated 😸