Skip to content

Fix ssdp #5750

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.

Sign up for GitHub

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 9 commits into from
Mar 21, 2019
Merged

Fix ssdp #5750

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1073d96
Merge pull request #1 from esp8266/master
Lan-Hekary Feb 10, 2019
ad58d97
Update ESP8266SSDP.h
Lan-Hekary Feb 10, 2019
30d0ca9
Update ESP8266SSDP.cpp
Lan-Hekary Feb 10, 2019
2855047
include 'uuid:' in format String and in flash
Lan-Hekary Feb 11, 2019
ab31f4d
Merge branch 'master' into Fix-SSDP
Lan-Hekary Feb 11, 2019
74a6938
Update ESP8266SSDP.cpp
Lan-Hekary Feb 11, 2019
529b57f
Merge branch 'master' into Fix-SSDP
Lan-Hekary Feb 11, 2019
bd47760
Merge branch 'master' into Fix-SSDP
devyte Mar 21, 2019
6e5f401
Merge branch 'master' into Fix-SSDP
d-a-v Mar 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions libraries/ESP8266SSDP/ESP8266SSDP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check this review :
#5750 (review)

"%s: %s\r\n" // "NT" or "ST", _deviceType
"LOCATION: http://%u.%u.%u.%u:%u/%s\r\n" // WiFi.localIP(), _port, _schemaURL
"\r\n";
Expand Down Expand Up @@ -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>"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above comment for USN.

"</device>"
//"<iconList>"
//"<icon>"
Expand Down Expand Up @@ -130,8 +130,10 @@ SSDPClass::SSDPClass() :
_timer(0),
_port(80),
_ttl(SSDP_MULTICAST_TTL),
_respondToAddr(0,0,0,0),
_respondToPort(0),
_pending(false),
_st_is_uuid(false),
_delay(0),
_process_time(0),
_notify_time(0)
Expand All @@ -155,12 +157,13 @@ SSDPClass::~SSDPClass() {

bool SSDPClass::begin() {
end();

_pending = false;
_st_is_uuid = false;
if (strcmp(_uuid,"") == 0) {
uint32_t chipId = ESP.getChipId();
sprintf(_uuid, "38323636-4558-4dda-9188-cda0e6%02x%02x%02x",
(uint16_t) ((chipId >> 16) & 0xff),
sprintf_P(_uuid, PSTR("uuid:38323636-4558-4dda-9188-cda0e6%02x%02x%02x"),
(uint16_t) ((chipId >> 16) & 0xff),
(uint16_t) ((chipId >> 8) & 0xff),
(uint16_t) chipId & 0xff);
}
Expand All @@ -178,7 +181,9 @@ bool SSDPClass::begin() {
IPAddress mcast(SSDP_MULTICAST_ADDR);

if (igmp_joingroup(local, mcast) != ERR_OK ) {
DEBUGV("SSDP failed to join igmp group");
#ifdef DEBUG_SSDP
DEBUG_SSDP.printf_P(PSTR("SSDP failed to join igmp group\n"));
#endif
return false;
}

Expand Down Expand Up @@ -241,7 +246,7 @@ void SSDPClass::_send(ssdp_method_t method) {
_modelNumber,
_uuid,
(method == NONE) ? "ST" : "NT",
_deviceType,
(_st_is_uuid) ? _uuid : _deviceType,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 ??

ip[0], ip[1], ip[2], ip[3], _port, _schemaURL
);

Expand Down Expand Up @@ -373,10 +378,19 @@ void SSDPClass::_update() {
#ifdef DEBUG_SSDP
DEBUG_SSDP.printf("REJECT: %s\n", (char *)buffer);
#endif
}else{
_st_is_uuid = false;
}
// if the search type matches our type, we should respond instead of ABORT
if (strcasecmp(buffer, _deviceType) == 0) {
_pending = true;
_st_is_uuid = false;
_process_time = millis();
state = KEY;
}
if (strcasecmp(buffer, _uuid) == 0) {
_pending = true;
_st_is_uuid = true;
_process_time = millis();
state = KEY;
}
Expand Down Expand Up @@ -416,6 +430,7 @@ void SSDPClass::_update() {
_send(NONE);
} else if(_notify_time == 0 || (millis() - _notify_time) > (SSDP_INTERVAL * 1000L)){
_notify_time = millis();
_st_is_uuid = false;
_send(NOTIFY);
}

Expand All @@ -439,7 +454,7 @@ void SSDPClass::setDeviceType(const char *deviceType) {
}

void SSDPClass::setUUID(const char *uuid) {
strlcpy(_uuid, uuid, sizeof(_uuid));
snprintf_P(_uuid, sizeof(_uuid), PSTR("uuid:%s"), uuid);
}

void SSDPClass::setName(const char *name) {
Expand Down
13 changes: 7 additions & 6 deletions libraries/ESP8266SSDP/ESP8266SSDP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ License (MIT license):

class UdpContext;

#define SSDP_UUID_SIZE 37
#define SSDP_UUID_SIZE 42
#define SSDP_SCHEMA_URL_SIZE 64
#define SSDP_DEVICE_TYPE_SIZE 64
#define SSDP_FRIENDLY_NAME_SIZE 64
Expand Down Expand Up @@ -66,17 +66,17 @@ class SSDPClass{
void setDeviceType(const String& deviceType) { setDeviceType(deviceType.c_str()); }
void setDeviceType(const char *deviceType);

/*To define a custom UUID, you must call the method before begin(). Otherwise an automatic UUID based on CHIPID will be generated.*/
void setUUID(const String& uuid) { setUUID(uuid.c_str()); }
void setUUID(const char *uuid);
/*To define a custom UUID, you must call the method before begin(). Otherwise an automatic UUID based on CHIPID will be generated.*/
void setUUID(const String& uuid) { setUUID(uuid.c_str()); }
void setUUID(const char *uuid);

void setName(const String& name) { setName(name.c_str()); }
void setName(const String& name) { setName(name.c_str()); }
void setName(const char *name);
void setURL(const String& url) { setURL(url.c_str()); }
void setURL(const char *url);
void setSchemaURL(const String& url) { setSchemaURL(url.c_str()); }
void setSchemaURL(const char *url);
void setSerialNumber(const String& serialNumber) { setSerialNumber(serialNumber.c_str()); }
void setSerialNumber(const String& serialNumber) { setSerialNumber(serialNumber.c_str()); }
void setSerialNumber(const char *serialNumber);
void setSerialNumber(const uint32_t serialNumber);
void setModelName(const String& name) { setModelName(name.c_str()); }
Expand Down Expand Up @@ -108,6 +108,7 @@ class SSDPClass{
uint16_t _respondToPort;

bool _pending;
bool _st_is_uuid;
unsigned short _delay;
unsigned long _process_time;
unsigned long _notify_time;
Expand Down