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.

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

Fix action POST #7

Merged
merged 17 commits into from
Aug 23, 2021
Merged

Fix action POST #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5dd83fa
Changed links in forms
trivo78 Jul 28, 2021
590f93a
Removed all "global" forms from TD
trivo78 Aug 5, 2021
e2413bc
removed global forms from server
trivo78 Aug 5, 2021
a2feda2
Fix misspelling of JsonObject
Citrullin Aug 11, 2021
1adc91b
Remove websocket
Citrullin Aug 11, 2021
30ebfd7
Move TD to /.well-known/wot-thing-description
Citrullin Aug 12, 2021
53d9023
Remove things array around TD
Citrullin Aug 12, 2021
924fe56
Remove commented out code
Citrullin Aug 17, 2021
1a674fa
Add w3c td v1 and webthings context
Citrullin Aug 17, 2021
123a053
Add things->forms for properties
Citrullin Aug 17, 2021
d5511fc
Remove Wifi101
Citrullin Aug 17, 2021
4f7a5a5
Update BPM280 example
Citrullin Aug 17, 2021
4350c1b
Fix Action POST
Citrullin Aug 18, 2021
98320fe
Add 500 Error when not able to create ThinActionObject
Citrullin Aug 18, 2021
442bc97
Add #ifdef ESP32 for WebThings, instead of ESPWebThing
Citrullin Aug 18, 2021
23ccab0
Remove all generic /actions/* endpoints
Citrullin Aug 18, 2021
8c2704b
clean memory on 500 error and remove unnecessary code
Citrullin Aug 19, 2021
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
267 changes: 14 additions & 253 deletions ESPWebThingAdapter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

#pragma once

#if defined(ESP32) || defined(ESP8266)


#include <ArduinoJson.h>
#include <ESPAsyncWebServer.h>
Expand Down Expand Up @@ -55,7 +55,7 @@ class WebThingAdapter {
}

MDNS.addService("webthing", "tcp", port);
MDNS.addServiceTxt("webthing", "tcp", "path", "/");
MDNS.addServiceTxt("webthing", "tcp", "path", "/.well-known/wot-thing-description");

DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods",
Expand All @@ -70,7 +70,7 @@ class WebThingAdapter {
this->server.on("/*", HTTP_OPTIONS,
std::bind(&WebThingAdapter::handleOptions, this,
std::placeholders::_1));
this->server.on("/", HTTP_GET,
this->server.on("/.well-known/wot-thing-description", HTTP_GET,
std::bind(&WebThingAdapter::handleThings, this,
std::placeholders::_1));

Expand Down Expand Up @@ -131,17 +131,7 @@ class WebThingAdapter {
std::bind(&WebThingAdapter::handleThingPropertiesGet,
this, std::placeholders::_1,
device->firstProperty));
this->server.on((deviceBase + "/actions").c_str(), HTTP_GET,
std::bind(&WebThingAdapter::handleThingActionsGet, this,
std::placeholders::_1, device));
this->server.on((deviceBase + "/actions").c_str(), HTTP_POST,
std::bind(&WebThingAdapter::handleThingActionsPost, this,
std::placeholders::_1, device),
NULL,
std::bind(&WebThingAdapter::handleBody, this,
std::placeholders::_1, std::placeholders::_2,
std::placeholders::_3, std::placeholders::_4,
std::placeholders::_5));

this->server.on((deviceBase + "/events").c_str(), HTTP_GET,
std::bind(&WebThingAdapter::handleThingEventsGet, this,
std::placeholders::_1, device));
Expand All @@ -158,15 +148,6 @@ class WebThingAdapter {
void update() {
#ifdef ESP8266
MDNS.update();
#endif
#ifndef WITHOUT_WS
// * Send changed properties as defined in "4.5 propertyStatus message"
// Do this by looping over all devices and properties
ThingDevice *device = this->firstDevice;
while (device != nullptr) {
sendChangedProperties(device);
device = device->next;
}
#endif
}

Expand All @@ -178,19 +159,6 @@ class WebThingAdapter {
this->lastDevice->next = device;
this->lastDevice = device;
}

#ifndef WITHOUT_WS
// Initiate the websocket instance
AsyncWebSocket *ws = new AsyncWebSocket("/things/" + device->id);
device->ws = ws;
// AsyncWebSocket * server, AsyncWebSocketClient * client, AwsEventType
// type, void * arg, uint8_t *data, size_t len, ThingDevice* device
ws->onEvent(std::bind(
&WebThingAdapter::handleWS, this, std::placeholders::_1,
std::placeholders::_2, std::placeholders::_3, std::placeholders::_4,
std::placeholders::_5, std::placeholders::_6, device));
this->server.addHandler(ws);
#endif
}

private:
Expand Down Expand Up @@ -228,118 +196,6 @@ class WebThingAdapter {
return false;
}

#ifndef WITHOUT_WS
void sendErrorMsg(DynamicJsonDocument &prop, AsyncWebSocketClient &client,
int status, const char *msg) {
prop["error"] = msg;
prop["status"] = status;
String jsonStr;
serializeJson(prop, jsonStr);
client.text(jsonStr.c_str(), jsonStr.length());
}

void handleWS(AsyncWebSocket *server, AsyncWebSocketClient *client,
AwsEventType type, void *arg, const uint8_t *rawData,
size_t len, ThingDevice *device) {
if (type == WS_EVT_DISCONNECT || type == WS_EVT_ERROR) {
device->removeEventSubscriptions(client->id());
return;
}

// Ignore all others except data packets
if (type != WS_EVT_DATA)
return;

// Only consider non fragmented data
AwsFrameInfo *info = (AwsFrameInfo *)arg;
if (!info->final || info->index != 0 || info->len != len)
return;

// Web Thing only specifies text, not binary websocket transfers
if (info->opcode != WS_TEXT)
return;

// In theory we could just have one websocket for all Things and react on
// the server->url() to route data. Controllers will however establish a
// separate websocket connection for each Thing anyway as of in the spec.
// For now each Thing stores its own Websocket connection object therefore.

// Parse request
DynamicJsonDocument newProp(SMALL_JSON_DOCUMENT_SIZE);
auto error = deserializeJson(newProp, rawData, len);
if (error) {
sendErrorMsg(newProp, *client, 400, "Invalid json");
return;
}

String messageType = newProp["messageType"].as<String>();
JsonVariant dataVariant = newProp["data"];
if (!dataVariant.is<JsonObject>()) {
sendErrorMsg(newProp, *client, 400, "data must be an object");
return;
}

JsonObject data = dataVariant.as<JsonObject>();

if (messageType == "setProperty") {
for (JsonPair kv : data) {
device->setProperty(kv.key().c_str(), kv.value());
}
} else if (messageType == "requestAction") {
for (JsonPair kv : data) {
DynamicJsonDocument *actionRequest =
new DynamicJsonDocument(SMALL_JSON_DOCUMENT_SIZE);

JsonObject actionObj = actionRequest->to<JsonObject>();
JsonObject nested = actionObj.createNestedObject(kv.key());

for (JsonPair kvInner : kv.value().as<JsonObject>()) {
nested[kvInner.key()] = kvInner.value();
}

ThingActionObject *obj = device->requestAction(actionRequest);
if (obj != nullptr) {
obj->setNotifyFunction(std::bind(&ThingDevice::sendActionStatus,
device, std::placeholders::_1));
device->sendActionStatus(obj);

obj->start();
}
}
} else if (messageType == "addEventSubscription") {
for (JsonPair kv : data) {
ThingEvent *event = device->findEvent(kv.key().c_str());
if (event) {
device->addEventSubscription(client->id(), event->id);
}
}
}
}

void sendChangedProperties(ThingDevice *device) {
// Prepare one buffer per device
DynamicJsonDocument message(LARGE_JSON_DOCUMENT_SIZE);
message["messageType"] = "propertyStatus";
JsonObject prop = message.createNestedObject("data");
bool dataToSend = false;
ThingItem *item = device->firstProperty;
while (item != nullptr) {
ThingDataValue *value = item->changedValueOrNull();
if (value) {
dataToSend = true;
item->serializeValue(prop);
}
item = item->next;
}
if (dataToSend) {
String jsonStr;
serializeJson(message, jsonStr);
// Inform all connected ws clients of a Thing about changed properties
((AsyncWebSocket *)device->ws)->textAll(jsonStr);
}
}
#endif

void handleUnknown(AsyncWebServerRequest *request) {
if (!verifyHost(request)) {
return;
Expand All @@ -362,16 +218,15 @@ class WebThingAdapter {
request->beginResponseStream("application/json");

DynamicJsonDocument buf(LARGE_JSON_DOCUMENT_SIZE);
JsonArray things = buf.to<JsonArray>();
JsonObject thing = buf.to<JsonObject>();
ThingDevice *device = this->firstDevice;
while (device != nullptr) {
JsonObject descr = things.createNestedObject();
device->serialize(descr, ip, port);
descr["href"] = "/things/" + device->id;
device->serialize(thing, ip, port);
thing["href"] = "/things/" + device->id;
device = device->next;
}

serializeJson(things, *response);
serializeJson(thing, *response);
request->send(response);
}

Expand Down Expand Up @@ -474,6 +329,7 @@ class WebThingAdapter {
void handleThingActionPost(AsyncWebServerRequest *request,
ThingDevice *device, ThingAction *action) {
if (!verifyHost(request)) {
Serial.println("Invalid Host");
return;
}

Expand All @@ -486,38 +342,21 @@ class WebThingAdapter {
new DynamicJsonDocument(SMALL_JSON_DOCUMENT_SIZE);
auto error = deserializeJson(*newBuffer, (const char *)body_data);
if (error) { // unable to parse json
Serial.println("Unable to parse JSON");
b_has_body_data = false;
memset(body_data, 0, sizeof(body_data));
request->send(500);
delete newBuffer;
return;
}

JsonObject newAction = newBuffer->as<JsonObject>();

if (!newAction.containsKey(action->id)) {
b_has_body_data = false;
memset(body_data, 0, sizeof(body_data));
request->send(400);
delete newBuffer;
return;
}

ThingActionObject *obj = device->requestAction(newBuffer);

ThingActionObject *obj = action->create(newBuffer);
if (obj == nullptr) {
b_has_body_data = false;
memset(body_data, 0, sizeof(body_data));
request->send(500);
delete newBuffer;
return;
memset(body_data, 0, sizeof(body_data));
request->send(500);
return;
Copy link
Owner

Choose a reason for hiding this comment

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

Can you expand why now are we returning a 404 where before this was a 500?

Copy link
Author

Choose a reason for hiding this comment

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

You are right. That's wrong. I wanted to find the ThingActionObject first and returned a 404 when it wasn't able to find it. Was a bit confused. Good catch. Fixed it.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok cool, don't we need to clear the memory as it was done previously?

Also, previously we had an if statement just above this check, looking for the presence of the action id. In my understanding, this is removed due to the fact that it was something specific from the Withings protocol. Now we comply with the data payload described in the TD. right?

Copy link
Author

@Citrullin Citrullin Aug 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I understand the function is only called when the URL already exist. So, the 404 should return before that. Just checked it. As it turns out, this doesn't happen. But this check has to happen somewhere else. I will take a look into it. It should send a 404.
In the previous implementation it checked, if the json object contains the action id, for some reason. Guess that has something to do with the Mozilla standard. That is different with the w3c standard. So, not necessary anymore.

Copy link
Author

Choose a reason for hiding this comment

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

So, it crashed before, because there was a generic handler for any /action/* endpoint, even if you didn't had a listener for it. I guess the Mozilla standard handled it different and returned an object with all available actions or something like that. I just removed it. You get the desired 404 now, if the action doesn't exist. Makes the code more simple as well.

Copy link
Author

Choose a reason for hiding this comment

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

I added the clean memory part. That's a good point. Not necessary, but good to do.
Yes, it now follows the TD payload structure and the webthings specific stuff is removed. Even though you can technically also define booleans etc. as input, but that cashes the code. Just added it as issue in #12

}

#ifndef WITHOUT_WS
obj->setNotifyFunction(std::bind(&ThingDevice::sendActionStatus, device,
std::placeholders::_1));
#endif

DynamicJsonDocument respBuffer(SMALL_JSON_DOCUMENT_SIZE);
JsonObject item = respBuffer.to<JsonObject>();
obj->serialize(item, device->id);
Expand Down Expand Up @@ -567,83 +406,6 @@ class WebThingAdapter {
request->send(response);
}

void handleThingActionsGet(AsyncWebServerRequest *request,
ThingDevice *device) {
if (!verifyHost(request)) {
return;
}
AsyncResponseStream *response =
request->beginResponseStream("application/json");

DynamicJsonDocument doc(LARGE_JSON_DOCUMENT_SIZE);
JsonArray queue = doc.to<JsonArray>();
device->serializeActionQueue(queue);
serializeJson(queue, *response);
request->send(response);
}

void handleThingActionsPost(AsyncWebServerRequest *request,
ThingDevice *device) {
if (!verifyHost(request)) {
return;
}

if (!b_has_body_data) {
request->send(422); // unprocessable entity (b/c no body)
return;
}

DynamicJsonDocument *newBuffer =
new DynamicJsonDocument(SMALL_JSON_DOCUMENT_SIZE);
auto error = deserializeJson(*newBuffer, (const char *)body_data);
if (error) { // unable to parse json
b_has_body_data = false;
memset(body_data, 0, sizeof(body_data));
request->send(500);
delete newBuffer;
return;
}

JsonObject newAction = newBuffer->as<JsonObject>();

if (newAction.size() != 1) {
b_has_body_data = false;
memset(body_data, 0, sizeof(body_data));
request->send(400);
delete newBuffer;
return;
}

ThingActionObject *obj = device->requestAction(newBuffer);

if (obj == nullptr) {
b_has_body_data = false;
memset(body_data, 0, sizeof(body_data));
request->send(500);
delete newBuffer;
return;
}

#ifndef WITHOUT_WS
obj->setNotifyFunction(std::bind(&ThingDevice::sendActionStatus, device,
std::placeholders::_1));
#endif

DynamicJsonDocument respBuffer(SMALL_JSON_DOCUMENT_SIZE);
JsonObject item = respBuffer.to<JsonObject>();
obj->serialize(item, device->id);
String jsonStr;
serializeJson(item, jsonStr);
AsyncWebServerResponse *response =
request->beginResponse(201, "application/json", jsonStr);
request->send(response);

b_has_body_data = false;
memset(body_data, 0, sizeof(body_data));

obj->start();
}

void handleThingEventsGet(AsyncWebServerRequest *request,
ThingDevice *device) {
if (!verifyHost(request)) {
Expand Down Expand Up @@ -709,4 +471,3 @@ class WebThingAdapter {
}
};

#endif // ESP
Loading