Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Support of other hardware #726

Closed
ildar opened this issue Oct 8, 2021 · 7 comments
Closed

Support of other hardware #726

ildar opened this issue Oct 8, 2021 · 7 comments
Labels
question/discussion This is just a question, for ex. not an issue or a feature request

Comments

@ildar
Copy link

ildar commented Oct 8, 2021

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
Copy link
Author

ildar commented Oct 8, 2021

kinda @JF002 's position here.

@Avamander Avamander changed the title [policy] other hardware support Other hardware support Oct 8, 2021
@Avamander Avamander changed the title Other hardware support Support of other hardware Oct 8, 2021
@Avamander Avamander added the question/discussion This is just a question, for ex. not an issue or a feature request label Oct 8, 2021
@geekbozu
Copy link
Member

geekbozu commented Oct 8, 2021

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
Copy link
Contributor

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
Copy link
Collaborator

JF002 commented Oct 9, 2021

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
Copy link

ck-telecom commented Nov 15, 2021

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
Copy link
Collaborator

easy
moving to Zephyr

:D

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

@ildar
Copy link
Author

ildar commented Jun 14, 2022 via email

@InfiniTimeOrg InfiniTimeOrg locked and limited conversation to collaborators Jan 22, 2023
@Avamander Avamander converted this issue into discussion #1557 Jan 22, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question/discussion This is just a question, for ex. not an issue or a feature request
Projects
None yet
Development

No branches or pull requests

6 participants