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

Revert FlashStringHelper Macros #8143

Conversation

mrengineer7777
Copy link
Collaborator

@mrengineer7777 mrengineer7777 commented May 1, 2023

Description of Change

Revert to previous definition of FPSTR and F macros.

Tests scenarios

Tested on arduino-esp32 core v2.0.8 with Adafruit Huzzah32 (Feather ESP32).
Tested on Win11 computer with VSCode+PIO.

#include <Arduino.h>

const __FlashStringHelper * testfunc(void)
{
  return F("test");
}

void setup() {
  Serial.begin(115200);
  
}

void loop() {
  String s;

  // put your main code here, to run repeatedly:
  delay(1000);
  s = testfunc();
  Serial.println(s);
}

Results:

ets Jul 29 2019 12:21:46

rst:0x1 (POWERON_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00       
mode:DIO, clock div:1
load:0x3fff0030,len:1184
load:0x40078000,len:13220
ho 0 tail 12 room 4
load:0x40080400,len:3028
entry 0x400805e4
test
test
test

Related links

Closes #8108

Revert to previous definition of `FPSTR` and `F` macros.
@mrengineer7777 mrengineer7777 added this to the 2.0.9 milestone May 1, 2023
@mrengineer7777 mrengineer7777 marked this pull request as ready for review May 1, 2023 15:42
@dok-net
Copy link
Contributor

dok-net commented May 2, 2023

This PR is a subset of #8111, just slightly easier to digest, but as such it fails to compile this (portable) MCVE, in contrast to the complete fix as proposed in #8111:

#ifdef ESP8266
#include <ESP8266WiFi.h>
#else
#include <WiFi.h>
#endif

const char ssid[] PROGMEM = "your-ssid";

void setup()
{
    WiFi.begin(FPSTR(ssid), F("your-password"));
}

void loop()
{
}

@me-no-dev
Copy link
Member

@dok-net in #8111 you are changing more things than necessary to achieve the object of reverting the breaking changes from your previous PR. We want to focus on that, before changing other libs.

@mrengineer7777 mrengineer7777 changed the base branch from release/v2.x to master May 2, 2023 13:23
@mrengineer7777 mrengineer7777 changed the base branch from master to release/v2.x May 2, 2023 13:24
@SuGlider
Copy link
Collaborator

SuGlider commented May 2, 2023

#8147 seems to be the same, just with a different branch target for merging.
I think that @me-no-dev will merge it on both...

Copy link
Collaborator

@SuGlider SuGlider left a comment

Choose a reason for hiding this comment

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

LGTM

@me-no-dev me-no-dev merged commit f89df42 into espressif:release/v2.x May 3, 2023
34 checks passed
@mrengineer7777 mrengineer7777 deleted the BugFix_FlashStringHelper_Macros branch May 3, 2023 17:48
me-no-dev pushed a commit that referenced this pull request Sep 4, 2023
Revert to previous definition of `FPSTR` and `F` macros.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants