Skip to content

Conversation

guppy0130
Copy link

Description of Change

  • introduce Logging Subsystem option in arduino IDE
    • introduces arduino and esp_idf options
  • esp_idf option sets the build.log_subsystem variable to -DUSE_ESP_IDF_LOG, which is passed into build.extra_flags
  • arduino option is an empty str and therefore no new flags are added to build.extra_flags
  • cannot use build.defines as that conflicts with the PSRAM -D flag?

note: this only adds it for XIAO ESP32S3; I can add it to the rest of the boards if that's desirable, but hoping y'all might have some automation for that.

Test Scenarios

Hardware: FQBN: espressif:esp32:XIAO_ESP32S3:DebugLevel=debug,PSRAM=opi,LoggingSubsystem=ESP_IDF
Software: whatever is pulled in by current main

Example sketch:

// define some custom logger fx
int syslogger(const char *format, va_list args) {
  char buffer[256];
  vsnprintf(buffer, sizeof(buffer), format, args);

  // some custom write (e.g., for me I wrote to a remote syslog endpoint)
  Serial.print(buffer);

  va_end(args);
  return strlen(buffer);
}

void setup() {
  esp_log_set_vprintf(syslogger);
}

void loop() {
  ESP_LOGD("my_app", "hello world");
}

Without this change, ESP_LOGD only goes to serial? with this change, syslogger is used correctly.

Related links

#4845 introduced USE_ESP_IDF_LOG

@CLAassistant
Copy link

CLAassistant commented Oct 5, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

github-actions bot commented Oct 5, 2025

Messages
📖 🎉 Good Job! All checks are passing!

👋 Hello guppy0130, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against c21d161

* introduce `Logging Subsystem` option in arduino IDE
  * introduces `arduino` and `esp_idf` options
* `esp_idf` option sets the `build.log_subsystem` variable to
  `-DUSE_ESP_IDF_LOG`, which is passed into `build.extra_flags`
* `arduino` option is an empty str and therefore no new flags are added
  to `build.extra_flags`
* cannot use `build.defines` as that conflicts with the PSRAM `-D` flag?
@guppy0130 guppy0130 force-pushed the arduino_ide_expose_logging_subsystem branch from 154e662 to c21d161 Compare October 5, 2025 04:38
@guppy0130 guppy0130 changed the title expose logging subsystem for arduino IDE feat(board): expose logging subsystem arduino IDE Oct 5, 2025
@me-no-dev
Copy link
Member

Anyone can do this at any time by adding -DUSE_ESP_IDF_LOG to build_opt.h in their sketch folder. You can also overwrite the default log output by setting ets_install_putc1 or ets_install_putc2. Therefore I am not in favor of this PR

@guppy0130
Copy link
Author

Anyone can do this at any time by adding -DUSE_ESP_IDF_LOG to build_opt.h in their sketch folder.

Oh, didn't realize this; tried this out and works as expected. Don't really mind if this is closed then.

Will note that the change introduced in the PR may make it easier/more obvious for end users though? Happy to amend this to a docs update if that's desirable.

What makes the current set of options valid and this one less so? Or is it the overhead of tracking/maintaining another option (totally understandable)?

@me-no-dev
Copy link
Member

The USE_ESP_IDF_LOG option was added to help users who compile Arduino as component in their ESP-IDF projects. It is not meant to be used in Arduino IDE, because we have no control over IDF logs there. We even translate IDF logs back to Arduino logs if some IDF source is compiled (from the sketch or library). In Arduino IDE we use only Arduino logging, because that is what we can control (enabled, level, output and so on). All Serial outputs (UART, USB, etc.) already have a way to be used as log output and it's very easy to add that functionality to other ones too (as I have proposed above).

@me-no-dev me-no-dev closed this Oct 6, 2025
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