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 ESP8266WebServer::sendContent_P and ESP8266WebServer::send_P with contentLength #753

Merged
merged 8 commits into from
Sep 18, 2015
Merged

Conversation

martinayotte
Copy link
Contributor

  • add ESP8266WebServer::sendContent_P and ESP8266WebServer::send_P with size_t contentLength argument to allow binary content to be send, such as images.
  • fix in WiFiClient::write_P to use memcpy instead of memccpy allow binary content to be send, such as images.

@@ -169,6 +169,13 @@ void ESP8266WebServer::send_P(int code, PGM_P content_type, PGM_P content) {
sendContent_P(content);
}

void ESP8266WebServer::send_P(int code, PGM_P content_type, PGM_P content, size_t contentLength) {
String header;
_prepareHeader(header, code, String(FPSTR(content_type)).c_str(), contentLength);
Copy link
Member

Choose a reason for hiding this comment

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

Ouch, you are calling c_str() method of a temporary String instance here. Better move String declaration into a separate line.

@martinayotte
Copy link
Contributor Author

Hi IGRR,
I've simply copied the code from Makuna, line #167, so if it need to be changed, it need to be done on both.

@igrr
Copy link
Member

igrr commented Sep 9, 2015

Oh okay, somehow i missed that. I'll merge everything then and do the necessary changes.

@martinayotte
Copy link
Contributor Author

Hi again Ivan,

I hate putting my hands/arms into a wringer, especially when it is not my own code.

During my previous tests, I forgot to add PSTR("image/png") in my calls.
Adding that, I got some crashes, even in Makuna original code.
So, I conclude that original code isn't working :

prepareHeader(header, code, String(FPSTR(content_type)).c_str(), contentLength);

Changing that to what you suggest me doesn't work either :

String type = String(FPSTR(content_type));
_prepareHeader(header, code, type.c_str(), contentLength);

So, I've think the proper way is again to use memccpy_P(), but I feel it is a bit ugly :

char type[64];
memccpy_P((void_)type, (PGM_VOID_P)content_type, 0, sizeof(type));
prepareHeader(header, code, (const char )type, contentLength);

I will commit that for now, but I think it needs to be revisited...
(All this simply to avoid content_type been a PROGMEM, as I think you suggested before, it was maybe useless)
I will appreciate you help here !

igrr added a commit that referenced this pull request Sep 18, 2015
add ESP8266WebServer::sendContent_P and ESP8266WebServer::send_P with contentLength
@igrr igrr merged commit b63c975 into esp8266:esp8266 Sep 18, 2015
igrr added a commit that referenced this pull request Oct 29, 2015
add ESP8266WebServer::sendContent_P and ESP8266WebServer::send_P with contentLength
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.

2 participants