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

Use bool in WString Return values #7766

Closed
1 task done
mrengineer7777 opened this issue Jan 27, 2023 · 4 comments · Fixed by #7774
Closed
1 task done

Use bool in WString Return values #7766

mrengineer7777 opened this issue Jan 27, 2023 · 4 comments · Fixed by #7774
Labels
Status: In Progress Issue is in progress Status: To be implemented Selected for Development Type: Feature request Feature request for Arduino ESP32

Comments

@mrengineer7777
Copy link
Collaborator

Related area

WString.h

Hardware specification

N/A

Is your feature request related to a problem?

I would like our WString library to return the same values as ESP8266 and ArduinoCore-API.

Describe the solution you'd like

I'd like to change function return values from unsigned char to bool. This was done in both ESP8266 (esp8266/Arduino#7939) and ArduinoCore-API (arduino/ArduinoCore-API#147)

Describe alternatives you've considered

I could leave the library as-is, but it makes it very hard to compare changes between ESP32 and ESP8266.

Additional context

I'm willing to submit a PR with the changes. I believe this will have no impact on users. Before I do the work I'd like feedback from repository maintainers that this will be helpful/accepted.

I have checked existing list of Feature requests and the Contribution Guide

  • I confirm I have checked existing list of Feature requests and Contribution Guide.
@mrengineer7777 mrengineer7777 added the Type: Feature request Feature request for Arduino ESP32 label Jan 27, 2023
@SuGlider
Copy link
Collaborator

ESP32 Arduino WString header files are exactly like Arduino mainstream has set.
https://github.com/arduino/ArduinoCore-avr/blob/master/cores/arduino/WString.cpp

This is an interesting request. Why did ESP8266 Arduino change that?

@SuGlider
Copy link
Collaborator

Nevermind, I see that all other Arduino Cores use it as bool and it has been already commited, as you pointed out.
I'm OK with this change. I think that others will also agree.

Please proceed with the PR.

@SuGlider
Copy link
Collaborator

BTW, I think that the closer ESP32 Arduino gets to ESP8266 Arduino, the better.

@VojtechBartoska VojtechBartoska added the Status: To be implemented Selected for Development label Jan 30, 2023
@VojtechBartoska
Copy link
Collaborator

Hello @mrengineer7777,

thanks for the suggestion. We've discussed it today within the Team and agree on this. Thanks for your help and we are looking for your PR including this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress Issue is in progress Status: To be implemented Selected for Development Type: Feature request Feature request for Arduino ESP32
Projects
Development

Successfully merging a pull request may close this issue.

3 participants