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.

Sign up for GitHub

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

Jump to bottom

Allow use of CDC/JTAG loggers on esp32 variants with Arduino #4658

Merged
merged 8 commits into from
Dec 14, 2023
Merged

Allow use of CDC/JTAG loggers on esp32 variants with Arduino #4658

merged 8 commits into from
Dec 14, 2023

Conversation

jesserockz
Copy link
Member

What does this implement/fix?

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040

Example entry for config.yaml:

# Example config.yaml

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@probot-esphome
Copy link

probot-esphome bot commented Apr 5, 2023

Hey there @esphome/core, mind taking a look at this pull request as it has been labeled with an integration (logger) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@probot-esphome probot-esphome bot removed the small-pr label May 8, 2023
This was referenced May 21, 2023
Merged
Closed
Fabian-Schmidt pushed a commit to Fabian-Schmidt/esphome that referenced this pull request May 23, 2023
Remove cpp & h changes to combine with PR esphome#4658
653c3e2
@Fabian-Schmidt
Copy link
Contributor

Any update on this PR?
It kind of blocks my ability to develop a solution with my ESP32-S3.

@github-actions
Copy link
Contributor

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 22, 2023
@github-actions github-actions bot closed this Sep 30, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 1, 2023
@kbx81 kbx81 reopened this Nov 11, 2023
@kbx81 kbx81 requested a review from a team as a code owner November 11, 2023 03:53
@kbx81 kbx81 added not-stale Won't go stale and removed stale labels Nov 11, 2023
@kbx81
Copy link
Member

kbx81 commented Dec 13, 2023

Ok, this should be good to go now -- just getting #4853 sync'd up with it.

kbx81
kbx81 reviewed Dec 13, 2023
View reviewed changes
esphome/components/logger/logger.cpp
Comment on lines +273 to +295
#if defined(USE_ESP32) && \
(defined(USE_ESP32_VARIANT_ESP32S2) || defined(USE_ESP32_VARIANT_ESP32S3) || defined(USE_ESP32_VARIANT_ESP32C3))
#if defined(USE_ESP32_VARIANT_ESP32S2) || defined(USE_ESP32_VARIANT_ESP32S3)
case UART_SELECTION_USB_CDC:
#endif // USE_ESP32_VARIANT_ESP32S2 || USE_ESP32_VARIANT_ESP32S3
#if defined(USE_ESP32_VARIANT_ESP32C3) || defined(USE_ESP32_VARIANT_ESP32S3)
case UART_SELECTION_USB_SERIAL_JTAG:
#endif // USE_ESP32_VARIANT_ESP32C3 || USE_ESP32_VARIANT_ESP32S3
#ifdef USE_ESP32_VARIANT_ESP32C3
this->hw_serial_ = &Serial;
Serial.begin(this->baud_rate_);
#endif // USE_ESP32_VARIANT_ESP32C3
#if defined(USE_ESP32_VARIANT_ESP32S2) || defined(USE_ESP32_VARIANT_ESP32S3)
#if ARDUINO_USB_CDC_ON_BOOT
this->hw_serial_ = &Serial;
Serial.begin(this->baud_rate_);
#else
this->hw_serial_ = &Serial;
Serial.begin(this->baud_rate_);
#endif // ARDUINO_USB_CDC_ON_BOOT
#endif // USE_ESP32_VARIANT_ESP32S2 || USE_ESP32_VARIANT_ESP32S3
break;
#endif // USE_ESP32 && (USE_ESP32_VARIANT_ESP32S2 || USE_ESP32_VARIANT_ESP32S3 || USE_ESP32_VARIANT_ESP32C3)
Copy link
Member

Choose a reason for hiding this comment

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

Some of these #ifdefs could be condensed -- I'm not sure if we want to keep them broken apart (in case things change in the future) or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha, its so hard to read (Not saying it has to change right now)

jesserockz
jesserockz commented Dec 14, 2023
View reviewed changes
Copy link
Member Author

@jesserockz jesserockz left a comment

Choose a reason for hiding this comment

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

✔️ (Cant approve my own PR 🤣) Tested and working on my s3-box-3 with arduino and even with improv_serial

@kbx81 kbx81 merged commit 777cdb1 into dev Dec 14, 2023
44 checks passed
@kbx81 kbx81 deleted the jesserockz-2023-136 branch December 14, 2023 04:24
@kbx81 kbx81 added this to the 2023.12.0b2 milestone Dec 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kbx81 kbx81 kbx81 approved these changes

Assignees
No one assigned
Labels
cherry-picked core integration: logger not-stale Won't go stale small-pr
Projects
None yet
Milestone
2023.12.0b2
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jesserockz @Fabian-Schmidt @kbx81