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

HTTP_UPDATE_OK naming collision #1169

Closed
2 of 4 tasks
markg85 opened this issue Dec 12, 2020 · 1 comment
Closed
2 of 4 tasks

HTTP_UPDATE_OK naming collision #1169

markg85 opened this issue Dec 12, 2020 · 1 comment

Comments

@markg85
Copy link

markg85 commented Dec 12, 2020

Hardware

WiFimanager Branch/Release:

  • Master
  • Development

Esp8266/Esp32:

  • ESP8266
  • ESP32

I'm adding update functionality to an ESP32 project. For that i'm using the HTTPUpdate.h file that comes with the ESP32 framework.
It, in it's header has this:

enum HTTPUpdateResult {
    HTTP_UPDATE_FAILED,
    HTTP_UPDATE_NO_UPDATES,
    HTTP_UPDATE_OK
};

typedef HTTPUpdateResult t_httpUpdate_return; // backward compatibility

This collides with the strings_en.h file (soft reminder why: C++ enum is without the class in between is global scope):

const char HTTP_UPDATE_OK[] PROGMEM = "<div class='msg P'><strong>Update OK!</strong> Device Rebooting now...</div>";

In my opinion both projects are wrong here.
WiFiManager should probably namespace these things as these names are definitely easy to use by other projects too.
And the ESP folks should have done enum class HTTPUpdateResult. Though adding that now adds a load more compile errors.

The "easiest" fix is to throw a namespace around those string messages. Though that does give a bit of pain in the rest of the WiFiManager source to fix the result of that namespace.

Other then that, i quite like WiFiManager now :) I had my issues with it in the beginning but ever since using the development branch it's - ironically - working super stable!

Cheers,
Mark

tablatronix added a commit that referenced this issue Dec 13, 2020
@tablatronix
Copy link
Collaborator

I thought I had an issue for namespacing, ill make one

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

No branches or pull requests

2 participants