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 1 commit
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
36 changes: 5 additions & 31 deletions ESPWebThingAdapter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ class WebThingAdapter {
void handleThingActionPost(AsyncWebServerRequest *request,
ThingDevice *device, ThingAction *action) {
if (!verifyHost(request)) {
Serial.println("Invalid Host");
return;
}

Expand All @@ -351,6 +352,7 @@ 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);
Expand All @@ -360,22 +362,10 @@ class WebThingAdapter {

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;
request->send(404);
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

}

DynamicJsonDocument respBuffer(SMALL_JSON_DOCUMENT_SIZE);
Expand Down Expand Up @@ -466,24 +456,8 @@ class WebThingAdapter {

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;
}

DynamicJsonDocument respBuffer(SMALL_JSON_DOCUMENT_SIZE);
JsonObject item = respBuffer.to<JsonObject>();
obj->serialize(item, device->id);
Expand Down
6 changes: 2 additions & 4 deletions Thing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ class ThingActionObject {
JsonObject data = obj.createNestedObject(name);

JsonObject actionObj = actionRequest->as<JsonObject>();
JsonObject inner = actionObj[name];
data["input"] = inner["input"];
data["input"] = actionObj;

data["status"] = status;
data["timeRequested"] = timeRequested;
Expand All @@ -111,8 +110,7 @@ class ThingActionObject {
setStatus("pending");

JsonObject actionObj = actionRequest->as<JsonObject>();
JsonObject inner = actionObj[name];
start_fn(inner["input"]);
start_fn(actionObj);

finish();
}
Expand Down