Skip to content

[SSDP] add schema(Print &) const #6798

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

Merged
merged 4 commits into from
Nov 19, 2019
Merged

Conversation

earlephilhower
Copy link
Collaborator

Supercedes #2806

Make SSDP::schema(WiFiClient&) use a by-ref (reduce stack use)

Add a SSDP::schema(Print&)

From @Palatis' original PR:
useful when using AsyncWebServer.

Supercedes esp8266#2806

Make SSDP::schema(WiFiClient&) use a by-ref (reduce stack use)

Add a SSDP::schema(Print&)

From @Palatis' original PR:
useful when using AsyncWebServer.
@earlephilhower
Copy link
Collaborator Author

@d-a-v, for the SSDP patch, I'm just the messenger.
I can't think of a good use case for a Printable vs. sending it over the wire, honestly.

However, maybe it makes sense to use just export Print& interface and drop the WiFiClient& one? WiFiClient inherits a Print& interface from Stream...

@earlephilhower
Copy link
Collaborator Author

Err, @devyte I meant to ping!

@devyte
Copy link
Collaborator

devyte commented Nov 17, 2019

maybe it makes sense to use just export Print& interface and drop the WiFiClient& one

Exactly why I asked. I suggest picking which of the two bodies looks best and keeping that one with Print arg.
Something that bothers me: the IP printout in both cases, one with the octets the other with IP2STR. I think the IPAddress::toString() method should be used instead, but that means updating the schema string, I think.

Because WiFiClient inherits a Print interface, replace the
::schema(WiFiClient&) with ::schema(Print&) which is source compatible
with existing code and allows the functionality requested in the initial
PR.

Use ip.toString() in the templates instead of breaking up the octets of
the address.
@earlephilhower
Copy link
Collaborator Author

Both comments addressed. SSDP will now use ip.toString() instead of octet extraction, and will export only the schema(Print&) which should work for any existing code as WiFiClient has a Print interface..

@earlephilhower
Copy link
Collaborator Author

Well, I guess C++ can't convert silently from a WiFiClient to Print for us. Multiple inheritance C++ classes might have such limitations (not like Java where there's a first-class interface type).

Any ideas?

/home/travis/build/esp8266/Arduino/libraries/ESP8266SSDP/examples/SSDP/SSDP.ino:29:32: error: no matching function for call to 'SSDPClass::schema(esp8266webserver::ESP8266WebServerTemplate<WiFiServer>::ClientType)'

       SSDP.schema(HTTP.client());

                                ^

/home/travis/build/esp8266/Arduino/libraries/ESP8266SSDP/examples/SSDP/SSDP.ino:29:32: note: candidate is:

In file included from /home/travis/build/esp8266/Arduino/libraries/ESP8266SSDP/examples/SSDP/SSDP.ino:3:0:

/home/travis/arduino_ide/hardware/esp8266com/esp8266/libraries/ESP8266SSDP/ESP8266SSDP.h:65:10: note: void SSDPClass::schema(Print&) const

     void schema(Print &print) const;

          ^

/home/travis/arduino_ide/hardware/esp8266com/esp8266/libraries/ESP8266SSDP/ESP8266SSDP.h:65:10: note:   no known conversion for argument 1 from 'esp8266webserver::ESP8266WebServerTemplate<WiFiServer>::ClientType {aka WiFiClient}' to 'Print&'

@devyte
Copy link
Collaborator

devyte commented Nov 18, 2019

Might be running into the 1-step implicit casting rule. I'll take a look at some point.

@devyte devyte self-assigned this Nov 18, 2019
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.

glad to see the crazy forwarder idea worked :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants