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

Add HTTP_HEAD to HTTPMethod and parse it #6413

Merged
merged 6 commits into from
Aug 17, 2019
Merged

Add HTTP_HEAD to HTTPMethod and parse it #6413

merged 6 commits into from
Aug 17, 2019

Conversation

ZakCodes
Copy link
Contributor

Adds HTTP_HEAD to the HTTPMethod enum and parses it from a String when reading a request.

Resolves #6412

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

Please compare against #3007 and update this PR with whatever is missing.

@ZakCodes
Copy link
Contributor Author

I've added all the changes from the merge request you referred me to.

I've added a conversion to string for the HTTP status code 418 because it was part of that merge request, but I didn't know if you wanted it or if you thought that it's just a waste of memory, so I've made sure to do it in a separate commit.

I don't have any strong opinion on this status code, but what if someone wants to use an ESP8266 to control a tea pot?

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

LGTM, even used tabs as required by the Arduino keywords.txt parser. Thx!

@ZakCodes
Copy link
Contributor Author

No problem, it's been a pleasure!

@earlephilhower earlephilhower merged commit 0dbb04e into esp8266:master Aug 17, 2019
@johnm545
Copy link
Contributor

This is causing builds to fail with tzapu's WiFiManager for me:

In file included from C:\Users\John\Documents\Arduino\hardware\esp8266com\esp8266/tools/sdk/libc/xtensa-lx106-elf/include/sys/stdio.h:6:0,

                 from C:\Users\John\Documents\Arduino\hardware\esp8266com\esp8266/tools/sdk/libc/xtensa-lx106-elf/include/stdio.h:63,

                 from C:\Users\John\Documents\Arduino\hardware\esp8266com\esp8266\cores\esp8266/Arduino.h:32,

                 from sketch\build_with_webserver_mcve.ino.cpp:1:

C:\Users\John\Documents\Arduino\hardware\esp8266com\esp8266/tools/sdk/libc/xtensa-lx106-elf/include/sys/pgmspace.h:25:130: error: 'const char HTTP_HEAD []' redeclared as different kind of symbol

   #define PROGMEM __attribute__((section( "\".irom.text." __FILE__ "." __STRINGIZE(__LINE__) "."  __STRINGIZE(__COUNTER__) "\"")))

                                                                                                                                  ^

C:\Users\John\Documents\Arduino\libraries\WiFiManager/WiFiManager.h:25:24: note: in expansion of macro 'PROGMEM'

 const char HTTP_HEAD[] PROGMEM            = "<!DOCTYPE html><html lang=\"en\"><head><meta name=\"viewport\" content=\"width=device-width, initial-scale=1, user-scalable=no\"/><title>{v}</title>";

                        ^

In file included from C:\Users\John\Documents\Arduino\libraries\WiFiManager/WiFiManager.h:17:0,

                 from C:\Users\John\Documents\Arduino\build_with_webserver_mcve\build_with_webserver_mcve.ino:2:

C:\Users\John\Documents\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266WebServer\src/ESP8266WebServer.h:33:39: error: previous declaration of 'HTTPMethod HTTP_HEAD'

 enum HTTPMethod { HTTP_ANY, HTTP_GET, HTTP_HEAD, HTTP_POST, HTTP_PUT, HTTP_PATCH, HTTP_DELETE, HTTP_OPTIONS };

                                       ^

exit status 1
Error compiling for board NodeMCU 1.0 (ESP-12E Module).

Trivial test sketch:

#include <ESP8266WiFi.h>
#include <WiFiManager.h>

void setup() {
  // put your setup code here, to run once:
  
}

void loop() {
  // put your main code here, to run repeatedly:

}

Reverting the commit or changing the name of the HTTP_HEAD in WiFiManager makes the errors go away

@johnm545
Copy link
Contributor

The problem is WiFiManager's naming choice, but WiFiManager is quite widely used

@d-a-v
Copy link
Collaborator

d-a-v commented Aug 29, 2019

@tzapu @tablatronix what do you suggest ?
we can rename this consistent global enum
you can rename your internal variable
other suggestions ?

@devyte
Copy link
Collaborator

devyte commented Aug 30, 2019

Looks like a name clash. please open a new issue, fill in the required info, and we'll track.
Notes:
Possible solutions:
Move webserver enum to enum class
Add namespace to WifiManager
Change WifiManager var names

@tablatronix
Copy link
Contributor

I will hotfix this, and try to add namespacing to development branch, time permitting.

@tablatronix
Copy link
Contributor

tablatronix commented Aug 30, 2019

I just pushed wifimanager 0.15-beta
This issue does not exists in WM development branch FYI

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.

'HTTP_HEAD' isn't declared in HTTPMethod
6 participants