Fix ssdp #5750
Fix ssdp #5750
Conversation
Fix SSDP bug: The response to M-Search Packet with ST field set to UUID should be with the UUID not the Device Type Integrated 'uuid:' prefix into the char array of the _uuid
Fix SSDP bug: The response to M-Search Packet with ST field set to UUID should be with the UUID not the Device Type Integrated 'uuid:' prefix into the char array of the _uuid
|
I was thinking of using Dynamic allocation or Strings to store the parameters instead of Fixed size array .. |
I can't know. |
|
no .. I mean |
Yes, maybe in another pull-request so one fix doesn't delay the other |
|
Using String will be a good idea, especially since #5690 (just merged) |
OK.. I will make another pull request after this pull request is merged with the new modifications .. |
| @@ -71,7 +71,7 @@ static const char _ssdp_packet_template[] PROGMEM = | |||
| "%s" // _ssdp_response_template / _ssdp_notify_template | |||
| "CACHE-CONTROL: max-age=%u\r\n" // SSDP_INTERVAL | |||
| "SERVER: Arduino/1.0 UPNP/1.1 %s/%s\r\n" // _modelName, _modelNumber | |||
| "USN: uuid:%s\r\n" // _uuid | |||
| "USN: %s\r\n" // _uuid | |||
There was a problem hiding this comment.
why is the prefix moved out of the template? it increases permanent heap use by 10 bytes due to the increased _uuid size, and I don't see a good reason for it. I prefer to keep the prefix here.
There was a problem hiding this comment.
check this review :
#5750 (review)
| @@ -99,7 +99,7 @@ static const char _ssdp_schema_template[] PROGMEM = | |||
| "<modelURL>%s</modelURL>" | |||
| "<manufacturer>%s</manufacturer>" | |||
| "<manufacturerURL>%s</manufacturerURL>" | |||
| "<UDN>uuid:%s</UDN>" | |||
| "<UDN>%s</UDN>" | |||
There was a problem hiding this comment.
Same as above comment for USN.
|
If you make a PR with Strings replacing the fixed-size arrays, please try to avoid concatenation, use direct construction or assignment when possible, and use ::reserve(). |
| @@ -241,7 +246,7 @@ void SSDPClass::_send(ssdp_method_t method) { | |||
| _modelNumber, | |||
| _uuid, | |||
| (method == NONE) ? "ST" : "NT", | |||
| _deviceType, | |||
| (_st_is_uuid) ? _uuid : _deviceType, | |||
There was a problem hiding this comment.
I moved the the prefix uuid: to the _uuid to avoid concatenating it in the line 249
alternative proposal would be
(_st_is_uuid) ? String("uuid:" + String(_uuid)).c_str() : _deviceType,
but this will introduce a bigger overhead I guess ..
do you have any other thoughts or alternatives I can use ??
Hello,
I noticed that the SSDP library always respond to the M-Search Packet with its Device type ..
and does not respond to UUID Search ..
such behavior prevents the other devices from querying the specific device (Such as Windows Network Discovery)
the Properties was returning an error beacuse of this bug
I fixed it without breaking compatibility and tested it thoroughly ..