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

Esp8266 oappend fix #4222

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Esp8266 oappend fix #4222

merged 2 commits into from
Oct 24, 2024

Conversation

willmmiles
Copy link
Collaborator

The oappend shim logic introduced in #4152 also has issues with SET_F(), as it casts away the helper type needed for print() to identify PROGMEM strings. Improve the shim logic to safely handle this case in two different ways:

  1. Detect PROGMEM addresses and re-apply the helper type, allowing the correct overload of print() to be called;
  2. Ask the platform to forgive us if we make a mistake, and handle it gracefully (if very slowly).

Fixes #4219.

Detect IRAM pointers if we can't be sure.
This is a platform feature that asks forgiveness for PROGMEM misuse:
it adds a handler such that incorrectly used PROGMEM will work without
crashing, just really, *really* inefficiently.

Given that most of our real-world use cases for PROGMEM strings are
relatively infrequent text calls, we can err on the side of developer
convenience and address performance problems if and when they arise.
@blazoncek
Copy link
Collaborator

@dosipod can you test this as well?
IMO it is good.

@dosipod
Copy link
Contributor

dosipod commented Oct 24, 2024

We tested both nodemcuv2_160 and esp8266_2m_160 .In both AR worked as expected and no more error or crash .

Copy link
Collaborator

@blazoncek blazoncek left a comment

Choose a reason for hiding this comment

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

Good as is.

@softhack007
Copy link
Collaborator

Good to merge 👍

@willmmiles willmmiles self-assigned this Oct 24, 2024
@willmmiles willmmiles merged commit 19d837c into 0_15 Oct 24, 2024
40 checks passed
@willmmiles willmmiles deleted the esp8266-oappend-fix branch October 24, 2024 19:23
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.

4 participants