InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.69k stars 919 forks source link

Support of other hardware #726

Closed ildar closed 1 year ago

ildar commented 2 years ago

nRF52 watches are numerous. Their essential parts are mostly similar:

  1. MCU bearing many essential functions (buttons, reset, power tracking)
  2. LCD display controllers are ST7789 for 240x240 and STxxxx (similar) for other resolutions.
  3. Touch sensors already diverge but still very similar to CSTx16S family.

Other peripherals (gyro, HR) are optional and can be added at later stages.

InfiniTime firmware has potential to work on many of the models. P8 port is already viable. Also pin abstraction patch already merged.

But to make InfiniTime truly open we need:

  1. official position.
  2. a policy on drivers.
ildar commented 2 years ago

kinda @JF002 's position here.

geekbozu commented 2 years ago

Just some additional feedback here, almost all of the driver code for the physical hardware is already separate. Supporting additional watches in forks should be just re-writing the Driver classes to expose the same api's working with the different hardware. This is currently out of the scope of what JF002 has the bandwidth to support. However it would be cool to see it ported around :D

hubmartin commented 2 years ago

Hi @ildar and others, I'm a really newbie in C++ but I would be curious what will be the best way to create a driver concept. For example accelerometer is passed to the SystemTask in the constructor with a type Bma421. https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/src/systemtask/SystemTask.cpp#L73

As I understand that this constructor passing is some kind of "dependency injection"?

What will be the best C++ approach to pass some generic sensor and connect it to the watches? Some Inheritance, or virtual functions? I'm sure #ifdefs are not a good option here. I have only experience in C where we use concept of drivers as a structure with function pointers (link below). But I'm sure in C++ might have some different approach/mechanism.

https://github.com/hardwario/twr-sdk/blob/f1c05143d8352e22066166c2bf8c37ad25e9b33c/twr/src/twr_ls013b7dh03.c#L136

I'm really just curious since I'm not experienced in this high-level object stuff. And wan't to learn more. Thanks.

JF002 commented 2 years ago

I'm not against supporting other hardware in InfiniTime, but I cannot maintain multiple ports by myself. Now that we have this github organization, we have the possibility to create teams and invite contributors that are willing to maintain ports for other hardware. Before we can actually support multiple hardware, I think the code needs a bit of re-organization/refactoring to make things smooth. This is something I thinking about for some time now, as i'm slowly doing some experiments with the BL60x/BL70x RISC-V MCUs :)

As I understand that this constructor passing is some kind of "dependency injection"?

Yes, this is dependency injection. It makes more sense when the type passed in parameter is a virtual type : the class expects to receive an object implementing an interface and never know that actual type that it'll be given. The caller is the only one that knows the actual type. In this case, the drivers are not virtual and does not inherit a virtual interface, but it still allow objects to be created at the very beginning of the execution and to be passed as reference to the rest of the code.

What will be the best C++ approach to pass some generic sensor and connect it to the watches? Some Inheritance, or virtual functions?

I try to avoid inheritance and virtual calls, as they add overhead and make the code less likely to be optimized. In fact, the main reason to avoid inheritance is because it's actually not needed : When Infinitime runs on a specific HW, there's no reason to discover which driver should be instantiated for this hardware, it's fixed and it'll never change. We will probably not build a single version of InfiniTime that is able to run on multiple hardware, as it'll make the binary size bigger for now reason, and we won't be able to support multiple MCU/architectures that way.

I also do not like #ifdef as they make the code hard to read and maintain.

I should be possible to use templates, constexpr c++20 concepts to ensure that the whole hardware and all the concrete types to be instantiated are known at build time. Another solution would be to use CMake to build only the files needed for each platform : let's say we rename Bma421 into MotionSensor. Then, we would have an implementation of this MotionSensor class in 2 separate folders : 1 for PineTime and 1 for P8. At build time, CMake would build the files from the PineTime folder to generate a binary for the PineTime, and then build the files from P8 to generate a firmware for the P8.

I'm still doing some experiment regarding this topics, so any suggestion is of course welcome!

ck-telecom commented 2 years ago

This issue is easy, just moving to Zephyr, it could support multi-boards with same code, and with a driver framework, and config could be used with each board, and hope a new version of powerfull hardware.

https://github.com/ck-telecom/pinetime

Avamander commented 2 years ago

easy moving to Zephyr

:D

The use of FreeRTOS is not really a significant hurdle on that path either IMO.

ildar commented 2 years ago

My initial suggestion was that @StarGate01 implemented in https://github.com/StarGate01/InfiniTime/commit/87a17435d2a0a6885befd49a470c27e7e815122d : having TARGET for different boards.

And believe me, there will be more to come.