make sure to check post vars too when a post request is done to the API

[dbursem]

made this to address https://github.com/serkri/SmartEVSE-3/issues/220

instead of doing the if (request->hasParam()) {request->getParam()} routine everywhere, I decided to make a method that checks both the post and get parameters, returns them if set or throws otherwise.

Still a draft because I haven't finished testing and still want to refactor a bit (e.g. throwing more specific exception).

I'm not very fluent in C++ (my native language is PHP) so LMK if there's anything I can learn to improve.

[dingo35]

I like your decision to make a separate method which tests for the parameters, it makes the changes on the existing code minimal, which I like!

Speaking of minimalizing changes, would it be possible to do the try/catch stuff in the new method, instead of it spreading around all the code?

BTW you have a typo in there FullSoc vs FullSoC

[dbursem]

I like your decision to make a separate method which tests for the parameters, it makes the changes on the existing code minimal, which I like!

Speaking of minimalizing changes, would it be possible to do the try/catch stuff in the new method, instead of it spreading around all the code?

that would defeat the purpose. I use the exception to indicate the new method was not able to fetch the desired parameter.

Technically I could leave the if/else routines in tact by making replacements for both hasParam and getParam, but since the analogy for getParam will have to do both anyway (since it needs to determine if its a GET or POST param first), it made more sense to me to just use the one method and use an exception to indicate if the param is not found.

edit: If there's other ways to achieve the same I'd like to learn them.

I could however undo some of the refactoring I did, to keep the changes more to the point.

BTW you have a typo in there FullSoc vs FullSoC

doh, should've compiled first sorry about that.

[dingo35]

Ok few remarks on your code here: 1) update your git (git pull), you are not working on the current master 2) your code does not seem to work. The "old" syntax still works: curl -X POST http://10.0.0.76/settings?override_current=150

but the "new¨, "normal" syntax doesn't: curl -H "Content-Type: application/json" -d "override_current":"150" http://10.0.0.76/settings curl -H "Content-Type: application/json" -d '{"override_current":"150"}' http://10.0.0.76/settings

3) Also I understand the "try/catch" statement is pretty expensive, in case of an exception; it saves the entire stack in case it has to roll back. And the general consensus is an exception as to "out of memory" is acceptable, an exception as to "bad user input" is not. The arduino is a pretty small processor, and we are constantly battling to reduce both CPU and memory usage. So I would like to avoid the try/catch construct....

[dbursem]

Thnx for the feedback! I have found a solution without the exception handling.

I did not test with a json content-type, I guess that's not supported by the AsyncWebServer library (at least not out of the box). It does however work when using the default application/x-www-form-urlencoded:

➜  SmartEVSE-3 git:(check-post-vars-on-api-calls) curl -v -d 'L1=11' -d 'L2=12' -d 'L3=13' -d 'battery_current=5' http://192.168.1.225/currents
*   Trying 192.168.1.225:80...
* Connected to 192.168.1.225 (192.168.1.225) port 80 (#0)
> POST /currents HTTP/1.1
> Host: 192.168.1.225
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 35
> Content-Type: application/x-www-form-urlencoded
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Length: 93
< Content-Type: application/json
< Connection: close
< Accept-Ranges: none
< 
* Closing connection 0
{"battery_current":5,"original":{"L":11,"":12,"SMART":13},"L":11,"":12,"SMART":13,"TOTAL":36}

Edit: This also makes a lot of JS in the frontend obsolete (can be replaced with regular HTML form submits) but I didn't want to go there (yet).

Not sure if I should also update the OpenAPI spec.

edit2: crap I think I messed up merging master, I think I'll close this MR and apply changes on a fresh branch (not going to bother rebasing/force-pushing)

[dbursem]

continued work in https://github.com/serkri/SmartEVSE-3/pull/229