Implemented compatibility layer

[jackjansen]

I think the implementation is as feature-complete as it's going to get (i.e.: it works for me:-). The fhessel branch is ready for merging into your repo, as soon as the bodyparser branch has been merged into esp32_https_server.

[fhessel]

Everything looks alright, so if there's nothing pending on your side, I'd merge the PR. Just some notes:

• Regarding d0d1353: If I want to build the example using pio run -e lolin32-FormServer -t upload from the library root, I need some modifications, too (mainly include Arduino.h and rename to .cpp). What you can do is to use pio ci to create a temporary project directory. I usually keep the .ino in git as its cleaner if you use the Arduino IDE.
• Is there a reason for keeping the #if 0 part in the examples? For the implementation, I see that you may use it to disable WIP code, but the examples should be as simple and intuitive as possible.

To publish the library, ...

• I'll adjust the versions in library.json and library.properties, and would switch the dependency to esp32_https_server~1.0.0 instead of pointing to the repo URL
• I'll change name in library.json to the repository name, ESP32 HTTP(S) Webserver (Compatibility Layer) is not really suitable for use in dependency lists
• Same goes for the name in library.properties

Then it should be ready to submit it to PIO and Arduino registries.

Any concerns? Otherwise just give me a go and I'll merge + publish.

[jackjansen]

Hmm, that ifdeffed out code is a bummer, because that is the code that actually implements the upload. I need to take a look and try to remember what I was actually thinking:-)

I also have a completely separate issue where the https server does not seem to work (ssl layer complains) in a program where Bluetooth is also enabled, I presume a memory shortage or something. I'm going to try and fix that too (if the issue is with this repo).

I will look into these and post a note here when I think it's ready for merging again.

BTW: I really like your CI with the nifty conversion of .ino into main.cpp, I need to adopt that too for other projects. I'm using the pio ci route at the moment but it doesn't work consistently. My intention was that by just keeping the .ino this repo would be ready for your CI method.

[fhessel]

Hmm, that ifdeffed out code is a bummer, because that is the code that actually implements the upload. I need to take a look and try to remember what I was actually thinking:-)

I ran the code on hardware and expected the upload to only work for the multipart POST upload in the HelloServer example. Since I got the file size in the serial output, I just assumed that to be the intended behavior.

I also have a completely separate issue where the https server does not seem to work (ssl layer complains) in a program where Bluetooth is also enabled, I presume a memory shortage or something.

Bluetooth is quite hungry for memory, so that may reduce the number of parallel connections that can be used with TLS. Also, there is only one hardware accelerator for crypto, so must not be accessed by the TLS server and e.g. Bluetooth running on a different core. But I would expect the ESP-IDF to take care of that (at least, mbedtls does this). OpenSSL afair doesn't return semantic error codes, so you don't know what went wrong.

BTW: I really like your CI with the nifty conversion of .ino into main.cpp, I need to adopt that too for other projects. I'm using the pio ci route at the moment but it doesn't work consistently. My intention was that by just keeping the .ino this repo would be ready for your CI method.

I couldn't use pio ci for the CI workflow, as I need some special project configurations (different lib_deps, adding certificate files to each project, ...) and I didn't want to add that to each example because that raises issues w.r.t. compatibility with the Arduino IDE. So for the moment, that seems to be the best solution. And it also allows parallelization of tests (building everything sequentially takes >30min, so you have to wait quite some time before you know whether your commit is okay).

[jackjansen]

The example was still showing too much of its test code origin. Fixed. It's still not an example that will win prizes, but I guess it'll do.

[jackjansen]

By the way, I thought of a way to allow your examples to be more convenient to use with CI for real testing, or during development using your private wifi parameters, without having to edit the ino files: if you replace the constructs

const char* ssid = ".........";
const char* password = ".........";

by something like

#ifndef ssid
const char* ssid = ".........";
#endif
#ifndef pasword
const char* password = ".........";
#endif

you could set the ssid/password for testing with a carefully constructed

build_flags=-Dssid=\""$CI_SSID"\" -Dpassword=\""$CI_SSID_PASSWORD"\"

line....

[fhessel]

I'll try the example again. If there aren't any problems, I'll merge it and prepare the release.

For the CI testing on hardware, I used this test repo to get familiar with different build systems. There, I had something running that copies the current password from a local hostapd-based access point into the project configuration (see here). That does pretty much what you suggest plus you don't have to sync a second password store.

In that project, I separated the CI apps there from the examples, as the intention of examples is to introduce the library to a new user, so we want to show its functions in small parts, feature by feature. I know that the HTTPS server isn't really beginner-friendly anyway, but I didn't want to put too much stuff/language features in there which aren't strictly necessary. For the CI applications, the situation and goal are different: These apps should cover as much functionality as possible to be able to test it.

I even had a Python helper script ready that allowed building and installing the apps on the fly and to integrate that with Python unittests so that each test suite would run only after the corresponding app was flashed and the ESP was reset after each test run.

That all worked well. However, sadly, I had to make the main repository with the self-hosted runner private again, as I cannot prevent someone from sending a new GitHub Actions Workflow with a new on push trigger in a PR to the repository, which then will be executed immediately on my Pi/ESP32 setup. The same goes for most other tools that I had a look at. They assume that the tests run in a sandbox/container, where a malicious PR cannot cause harm except for consuming resources.
If you know build systems that can be configured to a) keep runs for new PRs pending until they are started manually and b) use the configuration from the master branch to test the code in the PR branch, any hint would be greatly appreciated.

[fhessel]

Tested it and it seems to work. However, some small issues remain:

• The file param was still logged to serial output
• Some (probably copy+paste) issues with closing tags in the HTML markup of the example
• I added version-based library dependencies for the examples and the project
• I updated the license in library.json (I missed that when creating this lib from a template)

I added suggestions to fix everything. If you're okay with that, you can create a batch commit from all of them, then I'll merge the PR, change the name and version of the lib and release it.

[jackjansen]

If you know build systems that can be configured to a) keep runs for new PRs pending until they are started manually and b) use the configuration from the master branch to test the code in the PR branch, any hint would be greatly appreciated.

I'm not familiar enough with the GitHub actions. I use travis for CI/CD on GitHub, and until now I haven't done things like hardware tests on GitHub repos.

I have done this on gitlab (actually a private gitlab instance), and there I used the tags: section of the test-on-hardware to map that test to the specific runner (which had that tag too). And then I just made sure I didn't run the runner unless I wanted to. (In my case this was when the needed cameras were connected to the machine in question, but you could use the same idea to check which jobs would be run before starting the runner).

[fhessel]

I use travis for CI/CD on GitHub

Thanks for sharing the experience! I had a brief look at Travis, but it seems to be the same problem as with GitHub Actions w.r.t. code injection through PRs. I need to have a look at GitLab and see if/how it's solved there. Assigning tasks to specific runners works with GH Actions, too, but that doesn't prevent a third party from creating a new action with runs-on: self-hosted and submitting that in a PR. Then GitHub will happily send it to your machine.

For this PR, I'll just put it on hardware again to run it one last time (don't expect any issues, though) and then merge and create the release.

There's one thing left, which I meant to have already commented in the last batch, but I can't find it anymore: The license in library.json (MIT) differs from the one in the repo (LGPL-2.1-only, which is selected according to arduino-esp32). Do you have any concerns if I change it to LGPL?

[jackjansen]

There's one thing left, which I meant to have already commented in the last batch, but I can't find it anymore: The license in library.json (MIT) differs from the one in the repo (LGPL-2.1-only, which is selected according to arduino-esp32). Do you have any concerns if I change it to LGPL?

No problem at all.

[fhessel]

Hardware test was also successful.

[fhessel]

Added the build-examples action, submitted to PIO and to Arduino. They both require some kind of manual intervention, so it may take some days.

[fhessel]

PlatformIO is ready: https://platformio.org/lib/show/7221/esp32_https_server_compat