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.

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

WifiClient and WiFiClientSecure issues... #239

Open
nsavga opened this issue Jan 12, 2024 · 3 comments
Open

WifiClient and WiFiClientSecure issues... #239

nsavga opened this issue Jan 12, 2024 · 3 comments

Comments

@nsavga
Copy link

nsavga commented Jan 12, 2024

There are a some issues I encountered when coding for wb3s and wbr3 chips (using the feature/realtek-update branch for wbr3):

  • WiFiClientSecure doesnt work at all for any of the chips.
  • When using with wbr3 (RTL8720CF), WiFiClient class times out and stops when calling a GET with HttpClient and when having a big payload more than 2000~ bytes.
  • HttpClient class throws errors with time "multiple definition of `_gettimeofday' when building. it seems it is due to cookies part of the class..

But the good news is:

After some digging and experimenting, I solved all of the above issues by implementing:

**The only change I made in my local copy of LibreTiny repo is, changing the bool _connected field of LWIPClient class to protected instead of private, so that I could access it during inheritance.. All 3 classes I implemented rely on this change to work.

All of the above are packed in https://github.com/nsavga/WiFiLib-LibreTiny repository I created.

I tested them for both wbr3 and wb3s and everything seem to be working fine.

I checked the contributor's guide but I was still hesitant to create a pull request before communicating..
I would love to apply the changes, in an appropriate way, in LIBRETINY natively itself, and create a pull request.. but it would be better first discussing the most proper way to do it..
Also I am willing to contribute in any other part of the project after the discussion. (For instance, I desparately need to make wbr3(RTL8720CF) OTA work..)

@kuba2k2, please let me know how we can have a discussion for applying my solution into LibreTiny natively and possible future contributions..

Regards.

@kuba2k2
Copy link
Member

kuba2k2 commented Jan 12, 2024

Hi
We're more than open to any contributions.

I haven't had much time to check your library repo yet, so I just gave it a quick look.
Why do the two platforms need different implementations? They're both based on LwIP and mbedTLS, so I'm not sure why are the implementations so different.

If you want, we can discuss this further on our Discord server (link in the docs), or here of course. Feel free to make PRs, even if they don't get merged or will need changes I think it's worth it to show off your ideas.

I'm planning to refactor LibreTiny (again) and cleanup vendor SDKs as much as possible. It'll then use the same version of LwIP and mbedTLS on all platforms. That's a future plan, but I think it's reasonable and doable.

@nsavga
Copy link
Author

nsavga commented Jan 13, 2024

Hi,
Although they seem to have the same or similar versions of LwIP and mbedTLS, they have some differences and missing functions in both platforms in different points.
The non-TLS LwIP client had problems in mostly the available() function and also support issues for non-blocking functions.. Or at least bugs with them when handling bigger payloads..
On the mbedTLS side, again missing functions in different platforms. connect(), available() and read() functions of the WiFiClientSecure needed to be implemented slightly differently..

I ended up with two different classes because I already had the RTL version sorted out and when I needed to fix the Beken code I decided to first start from scratch to see what's going on, so I copied the ESP32 library's code as a start point. So I ended up with two sets of classes for different platforms. At the end, in any case, they need to be merged into one class for all platforms and handled with macros, but I didnt have time yet to polish and cleanup in that level.

My idea about the LibreTiny fixes for a pull request for the same area is:

  • Changing the HTTPClient class to accept IWiFiClient in its constructor instead of WiFiClient (so that the WiFiClientSecure can have its own code without inheriting the LwIPClient, as 99% of its code is separate logic.
  • Applying my fixes to both WiFiClient and WiFiClientSecure classes in LibreTiny and handling the platform differences with macros and ifndef logic in the same class..

If you agree with me about it, I will create a pull request with the above plan as soon as possible..

About the HTTPClient class issues. Do we really need the CookieJar implementation there?
If so, are you ok having another simpler class SimpleHTTPClient which doesnt have any redirection management, cookie and automatic 'switch to TLS' code? Smaller classes are better in most embedded cases as they are not having the unneeded overhead..

About the story on why I started to use your project:
I am working for a new IOT startup business. We initially made our module firmwares on ESP8266 and ESP32 chips.. Recently some Chinese manufacturers offered some products/modules which we are fitting in some of our use cases in our ecosystem, but having Beken or RTL chips. So I found your work when researching around, and started to test and implement our use cases with LibreTiny to support those chips..
After testing all in my list with a WBR3 chip which came with a test product, I ended up having two issues to handle: TLS and OTA.
With this code I finally managed handling the TLS issues. Now I need to find a way to make the OTA work too with the WBR3.. PWM and other stuff are not in good shape for WBR3 either, but luckily they are not needed at this moment for that particular product module's use cases..

I added you in LinkedIn and would like to talk to you.. In case you are willing to and have time, I would be happy to discuss about a plan to work together professionally.

The work we would do together would in any case end up as new features or bug fixes in LibreTiny, but with a professional, paid approach to work on issues/features..

Thanks and regards.

@kuba2k2
Copy link
Member

kuba2k2 commented Jan 13, 2024

We can accept such an approach (separate classes) as a temporary solution - until we cleanup the SDK and use the same versions of LwIP and mbedTLS on all platforms. This will probably happen sometime this year, but it might be months from now.

These classes (LwIPClient, WiFiClient[Secure] and HTTPClient) were copied from somewhere else, most likely ESP32 Arduino Core. They might be unstable, because I did not put much work into testing them. The reason for this is that LibreTiny is mostly used by ESPHome users, and not by developers working on their own projects. That makes it easy to miss such bugs, until someone actually starts using these libraries.

The HTTPClient class was also copied from other Arduino cores, because LibreTiny was meant to be ESP32-Arduino-compatible. Hence, removing a functionality (the CookieJar) could cause problems for some use cases, even if that functionality isn't 100% bug-free. Adding a new "simple" class is a better idea.

That being said, if you're doing more serious work with LibreTiny, consider using LwIP and mbedTLS directly - without the Arduino wrapper classes. That way you get much more control about what your program is doing. The LwIP library allows to use Unix socket-like approach to network programming.

Anyways, I've accepted the invitation on LinkedIn. I'm not actively using that platform, so I'm not sure where to begin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants