InfiniTime
InfiniTime copied to clipboard
P8: Mirror display for P8b variant
This PR has been broken out of https://github.com/InfiniTimeOrg/InfiniTime/pull/1050, and depends on / includes https://github.com/InfiniTimeOrg/InfiniTime/pull/1128 .
A new P8b hardware variant requires the display to be mirrored, and the colors to be inverted. Patch provided by @izzeho .
Related: https://github.com/StarGate01/p8b-infinitime/issues/3
@JF002 I like the idea of reducing #ifdefs
. I used them because they require minimal changes to the code and classes, but I agree that they kind of break the OOP model.
How would you go about writing these variant-specific header files? A file (e.g. src/variants.h
), which essentially does a similar selection as the CMake config (https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/src/CMakeLists.txt#L789), i.e. check TARGET_DEVICE
, and then define the needed constexpressions, similar to the Pins in drivers/PinMap.h
?
I dont think we can get rid of the CMake config completely, because it sets definitions for e.g. the NRF SDK as well.
This could also be used for control of the touch driver behaviour (https://github.com/InfiniTimeOrg/InfiniTime/pull/1130#discussion_r910661435), where @Riksu9000 was rightfully hesitant on littering the code with #ifdefs
as well.
The acceleration driver (https://github.com/InfiniTimeOrg/InfiniTime/pull/1132) is already abstracted and split into multiple classes, so a simple flag can be used to select which implementation to use. I believe link-time-optimization will then remove the unneded code.
@StarGate01 Sorry for the late answer! I have already investigated some ways to support multiple hardware using a more OOP approach, but I haven't finished those researches yet. The idea was to find a way to correctly encapsulate device drivers so that it's easy to add alternative implementations and support multiple hardware (simulator, other smartwatches like the P8, experiments with the PineDio STACK,...).
I'll try to remember what I've already done (I mean... find the git stash with the code :D) and so we can move forward on that topic!
@JF002 regarding encapsulation, what do you think of the approach of #1132 ? There I split the implementations of the device-specific drivers into two specifications of an abstract class. That would be quite a bit of overkill for this PR imho.
I will prepare a variants.h
approach as mentioned above, then we can discuss that ?
I think altering a single line with #ifdefs is simple and acceptable. In the case of functions, two or more functions can be defined and #ifdefs only choose which one gets called, which is what I would do in #1130 for example.
Yes, you are absolutely right @Riksu9000, those changes are pretty limited and harmless for the global architecture, and I think we can merge them.
I'm pretty sure we can find a better design than this to support multiple hardware variants, but I unfortunately don't have the time to work on that right now.
So, yeah, let's not block time any further. We'll still be able to improve the design in the future :)