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

LibreTiny refactor #278

Open
kuba2k2 opened this issue Apr 11, 2024 · 14 comments
Open

LibreTiny refactor #278

kuba2k2 opened this issue Apr 11, 2024 · 14 comments
Labels
BK7231 Beken BK72xx family bug Something isn't working common Common for all families enhancement New feature or request help wanted Extra attention is needed quality Code quality or safety improvements RTL8710B Realtek RTL87xxB family

Comments

@kuba2k2
Copy link
Member

kuba2k2 commented Apr 11, 2024

The introduction

As we're slowly approaching LibreTiny's 2nd birthday, it's time for a new v2.0.0 release with yet another refactor... that's how it works, right?

In this issue I'll try to document the ideas for the refactor. The proposed changes should mainly improve the workflow for developers of LibreTiny. They are also intended to make the project usage easier for end users.

I will gladly accept any feedback from a developer point of view, if anyone wishes to share.

Note that this is not a support thread, please don't post issues here.

The plan

  1. Components - the SDK should be refactored to move code to separate components. It seems to be the go-to solution in many modern SDKs, because it is easily extensible and helps keep the project clean and tidy.

    For example, the following components could be added: lt_core, lt_gpio or lt_io, lt_wifi, lt_uart, lt_i2c. Also, ports of external libraries would be kept in components too: lwip, printf, flashdb, etc.

    Each component would contain its source files as well as header files. They should be in the same directory (optionally headers in an include directory). If a component has different code on each family, there are several possibilities (up to further consideration):

    • per-family components: components/bk72xx/lt_gpio/lt_gpio.c and components/rtl8710b/lt_gpio/lt_gpio.c
    • per-family directories: components/lt_gpio/bk72xx/lt_gpio.c and components/lt_gpio/rtl8710b/lt_gpio.c
    • per-family files: components/lt_gpio/lt_gpio_bk72xx.c and components/lt_gpio/lt_gpio_rtl8710b.c
  2. The external libraries should also be reorganized. They should be packaged as real PlatformIO libraries (optionally published to PIO Registry), instead of the current way of using Git repos.

  3. Family names - currently, there are several different names for each family - name, code, short name, etc. For example, realtek-ambz, ambz, RTL8710B. It could be better to get rid of these names altogether, replacing them with a short name/code related to the chip name (like the LibreTiny components in ESPHome).

  4. LibreTiny C API - along with the components, the API could be refactored a bit. There are several confusing functions or unnecessary ones, that should be removed or placed somewhere else.

  5. Generic boards - should be renamed. Using generic-rtl8710bn-2mb-788k is pretty tedious and silly. Keeping a single generic board per-chip is more than enough.

  6. But how, generic boards with no flash size??
    Yes - the partition layout should be moved to a separate property. A directory called boards/partitions/ could contain predefined partition layouts for each family, e.g. bk7231n-beken.json or rtl8710b-tuya.json.
    The partition layout will be then assigned to a property in board JSON, with the option to change the chosen layout in platformio.ini (as well as in ESPHome YAML).
    The same could apply to e.g. Beken encryption keys.

  7. The Arduino Core should not be the priority. Currently, many important features (Wi-Fi, Serial, I2C, SPI) only work when using the Arduino Core, making it impossible to use all of LibreTiny for some users. The missing features in the C framework should be implemented as components.

  8. The Arduino libraries and the Core should then be replaced with thin wrappers around the LibreTiny API.

  9. Care must be taken when designing the new APIs for all peripherals. ESP-IDF or the mbed frameworks are good examples for inspiration. The APIs must be universal and designed to work with all features of the various microcontrollers.

  10. EDIT 2024-04-14: Remove FlashDB and FAL. Design an easier to use, lightweight KVS store, possibly compatible with FlashDB's storage format (it's easy enough I guess, yet the library is quite convoluted).

  11. Are there any other small and simple KVS libraries? I haven't found any.

  12. EDIT 2024-05-19: Migrate to GNU++17, if possible.

  13. Implement some kind of VFS to ultimately support LittleFS and/or SPIFFS. The VFS could ideally be UNIX-like, with a simple layer for mount points. A path like /dev could contain block devices (such as flash, eFuse, ROM, RAM or EEPROM emulation).

  14. Optionally, it could even contain character devices, such as SPI, I2C, UART, etc. If so, there could be a simple ioctl layer for these in order to provide functionality similar to the Linux kernel. It is debatable whether this should be 1) the only API for accessing these peripherals, 2) the main API with procedural wrappers or 3) procedural API with an optional devfs access layer.

  15. The VFS should support adding mount points with custom read/write routines, as well as adding custom block devices.

To be continued...

The other plan

Getting rid of the vendor SDK. At least as much as possible.

Many vendor SDKs package lots of unnecessary "features", such as:

  • Network protocols like MQTT, WebSocket, MDNS, HTTP, TFTP, etc. They're all either available in lwIP or unnecessary.
  • MP3 decoders, some weird demos, other libraries. We don't need these.

I also want to remove the following libraries from the SDKs and add clean ports in LibreTiny:

  • lwIP
  • mbedTLS
  • FreeRTOS
  • anything else?

It would be great to separate the Wi-Fi layer from lwIP. The SDK code should only worry about the physical layer, NOT the data link layer (and above) that are supported by lwIP. That means no IP addresses, DHCP or DNS support in the SDK. lwIP is perfectly capable of handling that on its own, that's how the TCP/IP and OSI layer models are supposed to work.

The wpa_supplicant (used in BDK, at least) might be separated and cleaned up. Not sure if that's possible, though.

Finally, to implement Bluetooth (BLE) support, BTstack could be used. It would provide a vendor-agnostic set of APIs for working with everything Bluetooth, as long as the SoC has an HCI transport layer.

That's it

For now

@kuba2k2 kuba2k2 added bug Something isn't working enhancement New feature or request help wanted Extra attention is needed BK7231 Beken BK72xx family RTL8710B Realtek RTL87xxB family quality Code quality or safety improvements common Common for all families labels Apr 11, 2024
@Cossid
Copy link
Collaborator

Cossid commented Apr 11, 2024

This all sounds very good, if not a bit lofty, so we must be careful, because as the saying goes, perfect is the enemy of good. One thing this plan misses is how to transition. I'd recommend focusing on wrapping up all open PRs first, as there are a lot of good improvements already there that should be made available and have a known-working comparison to for refactoring. Then maybe a PR freeze as a migration phase is worked through.

Once a PR freeze is in place, it would be good to break things into sections with separate issues, so that hopefully all the burden doesn't fall on you alone, and encourage participation of others as well.

@TuanAIoT
Copy link

TuanAIoT commented Apr 12, 2024

Very wonderful, thank you!
I hope BLE will be deployed soon 😂

@szupi-ipuzs
Copy link
Contributor

My knowledge of libretiny is still minimal, so my advices for the refactor will be of general type, based on the parts that I've seen and their "code-smells".

  • Get rid of #ifdefs as much as possible. Especially the ones that cut through a source file in several places. It's much better in such cases to have two (or more) similar files and compile them conditionally. Three reasons for this:
  1. as your project progresses and you keep adding more and more of new #defines, you eventually find yourself in a place that you can't predict what effect various combinations of #defines will make and if they even can compile.
  2. The code is simply much easier to read and analyze when you don't constantly need to think if a specific part of code applies to the case you're analyzing.
  3. Unit-testing is in general very hard when the code consists of many #ifdefs.
  • Group common static objects into structures. This c++-like coding style pays off even in plain-C very quickly, as it makes it much easier to keep the code clean or move it around.

  • Add some unit tests. I don't know if and how this is possible with platformio libs like yours, but I'm sure that with some effort, at least some layers of libretiny code can be unit-tested. Of course the #ifdefs will make it much harder. Still there's a big value in unit tests, not only they help you find bugs, but they make others less scared to break something, so this should add some new devs to the project :)

@rishabmehta7
Copy link

I hope the Project is not abandoned.

@kuba2k2
Copy link
Member Author

kuba2k2 commented Jul 8, 2024

I hope the Project is not abandoned.

It is not.
But please keep in mind, that being a single developer building this project, I might not have enough time for everything, while also having work and other projects (there is also personal life on top of that).
LibreTiny is a big project, and usually when I choose to commit to working on it for some time, I can't really work on anything else simultaneously.

@rishabmehta7
Copy link

I hope the Project is not abandoned.

It is not. But please keep in mind, that being a single developer building this project, I might not have enough time for everything, while also having work and other projects (there is also personal life on top of that). LibreTiny is a big project, and usually when I choose to commit to working on it for some time, I can't really work on anything else simultaneously.

Just a suggestion, Why cant the other ESPHome Devs help with LT? I am not a dev otherwise I would love to contribute.

@kuba2k2
Copy link
Member Author

kuba2k2 commented Jul 8, 2024

Because they are busy working on ESPHome. From what I understand, some lead developers of ESPHome actually have it as their full time job. I am unfortunately not that lucky and this is only my free time hobby project.

@rishabmehta7
Copy link

We should start a campaign for LT to be adopted by Open Home Foundation.

@kuba2k2
Copy link
Member Author

kuba2k2 commented Jul 8, 2024

I'm not sure how would that work and if it's even possible. The Open Home Foundation owns and maintains really big projects, like Home Assistant and ESPHome, LibreTiny probably doesn't fit that category.

@rishabmehta7
Copy link

I have just sent an email to them, let's see if something comes out of it.

Rishab

@xiaopeng12138
Copy link

I'm not very familiar with the current code base, but I think it's possible to merge this project with openbk.

Like build some kind of abstraction layer or base layer that can be used by openbk and esphome (application layer). Then only one repository needs to be maintained and the feature for openbk can also be used in esphome.

The people behind openbk can also help with development. And the project is more likely to be adopted by OHF since the community is larger.

Just a personal opinion, pretty idealistic.

@rishabmehta7
Copy link

Even though I don't like the idea of combining LT and OpenBK, as that would be like merging Tasmota with ESPHome. But I think it might be a good idea. With more people contributing, we might have a better chance of getting adopted by OHF.

@dwmw2
Copy link
Contributor

dwmw2 commented Oct 4, 2024

I think collaborating on the forks of the Beken BDK instead of diverging would probably make some sense, at least.

@xiaopeng12138
Copy link

I don't mean to merge entirely project of course. Just the core components/libs that make sense to merge.

And this is just a idea and has many difficulties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BK7231 Beken BK72xx family bug Something isn't working common Common for all families enhancement New feature or request help wanted Extra attention is needed quality Code quality or safety improvements RTL8710B Realtek RTL87xxB family
Projects
None yet
Development

No branches or pull requests

7 participants