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

Add software serial based logging (implements #33) #34

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

maehw
Copy link
Collaborator

@maehw maehw commented Oct 28, 2022

This pull request

  • introduces a new class called Logging based on base class Print and therefore
  • allows to use methods such as Logging::println().
  • It makes use of NeoSWSerial as other software serial impementations are not known to be compatible with the already used PinChangeInterrupt library. Make sure to #define NEOSWSERIAL_EXTERNAL_PCINT // uncomment to use your own PCINT ISRs inside of NeoSWSerial.h.
  • Logging can be also deactivated commenting #define LOGGING_ACTIVE inside of Logging.h (also not requiring to resolve the NeoSWSerial dependency). Probably should be the default as it currently is in the state of this pull request.

Side effects:

  • By default the constant strings seem to use SRAM memory. With many debug logs, this can easily fill up your SRAM (which needs to be shared along with stack memory). I have seen side affects where adding more log strings will make the program not run properly, probably due to stack overflow (I saw this happen using Arduino Uno). Using the F("mystring") macro, puts it in PROGMEM therefore not taking away "expensive" SRAM. Seems to be an appropriate solution.
    (Edit: see also https://www.arduino.cc/reference/en/language/variables/utilities/progmem/)
  • The call of Logging functions is not free of cost: some delay will be introduced by the function calls. Not sure if this has any side effects on real-time execution/ time critical functionalities in some of the SelfomatController firmware, e.g. when triggering the flash or playing animations on the NeoPixel ring. This would require some testing with the xtech hardware which I don't have available.

Please note that the code also isn't fully tested yet on my side - but at least provides a proof-of-concept (PoC).

@maehw maehw added the enhancement New feature or request label Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants