InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

Add initial P8 a/b smartwatch support

Open StarGate01 opened this issue 2 years ago • 14 comments

Since this PR was too large, it was split smaller, separate PRs. Same-level entries can be merged independent from each other.

  • https://github.com/InfiniTimeOrg/InfiniTime/pull/256
    • https://github.com/InfiniTimeOrg/InfiniTime/pull/1051
  • https://github.com/InfiniTimeOrg/InfiniTime/pull/1052
  • https://github.com/InfiniTimeOrg/InfiniTime/pull/1121
    • https://github.com/InfiniTimeOrg/InfiniTime/pull/1125
  • https://github.com/InfiniTimeOrg/InfiniTime/pull/1145
  • https://github.com/InfiniTimeOrg/InfiniTime/pull/1128
    • https://github.com/InfiniTimeOrg/InfiniTime/pull/1130
    • https://github.com/InfiniTimeOrg/InfiniTime/pull/1200
    • https://github.com/InfiniTimeOrg/InfiniTime/pull/1204
    • https://github.com/InfiniTimeOrg/InfiniTime/pull/1132
      • https://github.com/InfiniTimeOrg/InfiniTime/pull/1133

#1133 also requires #1145 for the BMA421.

PRs / WIPs in related projects:

  • https://github.com/InfiniTimeOrg/pinetime-mcuboot-bootloader/pull/10
  • https://github.com/apache/mynewt-core/pull/2798
  • https://github.com/apache/mynewt-core/pull/2858
  • https://github.com/apache/mynewt-nimble/issues/1207

This PR extends the existing P8 smartwatch support (eg. from https://github.com/InfiniTimeOrg/InfiniTime/commit/e614af1c4a53e943256b3b2acee4bfe7d7204bbf) to support models of the P8 series: P8a and P8b. This is a feature desired by the community (see https://github.com/InfiniTimeOrg/InfiniTime/issues/62).

The P8 smartwatches and the Pinetime have a few small differences:

  • Slightly different pins are used
  • Some variants have a SC7A20 acceleration sensor, some have a BMA421 (like the Pinetime)
  • Some variants have a CST716 touch screen sensor, some have a CST816S (like the Pinetime)
  • Some variants have no external low frequency crystal
  • Different SPI flash chips are used (Not relevant as no whitelist is enforced in InfiniTime)

I added support for the SC7A20 sensor based on the work by @hubmartin and @ildar at https://github.com/ildar/InfiniTime/tree/p8 . A CMake variable (DRIVER_ACC) can be used to select which sensor is used at compile time. The acceleration driver code was split into an generic interface definition and two hardware-specific implementations.

In addition, both acceleration drivers now use the internal FIFO buffer of the sensor to read high-frequency data without additional CPU load. This buffer is also made available via the motion data BLE service.

In addition, the touch driver can be configured for the touch sensor chip used, to adjust its logic accordingly. The CST716 does not support an automatic sleep state, and cannot wake from the explicit manual sleep state. On these devices, the touch sensor is kept awake if tap to wake is enabled. The CST816S is not explicitly put to sleep, instead it uses its auto-sleep functionality. On my Pinetime dev kit, I had some issues concerning the initialization of the touch sensor after a reset, so I implemented a small workaround for variants with that issue as well.

The CSTX16 touch sensor series can be used in two modes - reporting mode and gesture mode - which control when and how often an event is reported to the main CPU. The InfiniTime code currently assumes that both the CST816S as well as the CST716 are configured in reporting mode. The CST816S is able to switch at runtime to this mode, which is what the code does. The CST716 can only be configured once in the factory. In my P8b it came in reporting mode, however there are P8a variants with a CST716 stuck in gesture mode. The touch sensor mode can be specified by setting the CMake variable DRIVER_TOUCH.

The Nimble Bluetooth stack has been slightly modified to be compatible with the LFRC clock source calibration (see https://github.com/apache/mynewt-nimble/issues/1207), as well as the power-conserving BLE_LL_RFMGMT_ENABLE_TIME Nimble configuration option (see https://mynewt.apache.org/v1_7_0/network/ble_setup/ble_lp_clock.html#crystal-settle-time-configuration).

A second new CMake option (LF_CLK) can be used to specify which source should be used for the low frequency clock (used for Bluetooth). If the RC source is chosen, the code will periodically recalibrate the internal RC oscillator using the high frequency clock as a reference. And the third new option (TARGET_DEVICE) replaces WATCH_COLMI_P8 to select the pin map.

All these now options are optional and documented in buildAndProgram.md. By default, the regular Pinetime configuration is built. The CMake files were slightly refactored as well, for a better display of the configuration options and better debugging configuration.

A related PR was made to the bootloader: https://github.com/InfiniTimeOrg/pinetime-mcuboot-bootloader/pull/10 .

I realize this is a rather large and broad PR, so please feel free to criticize, and I'll be happy to discuss and incorporate any improvements.

StarGate01 avatar Mar 25 '22 10:03 StarGate01

This PR has been rebased onto https://github.com/InfiniTimeOrg/InfiniTime/pull/1052 , which in turn was rebased onto https://github.com/InfiniTimeOrg/InfiniTime/pull/1051 . Please review / merge those first.

StarGate01 avatar Mar 25 '22 12:03 StarGate01

@StarGate01 Thanks for all the awesome work! I'll try that on my P8 soon. I'm glad I could update them after long time.

hubmartin avatar Mar 25 '22 12:03 hubmartin

@hubmartin Thanks! Let me know how the testing goes, especially if you have a P8a watch (which I can't test on at the moment).

StarGate01 avatar Mar 25 '22 12:03 StarGate01

Merged all 3 PRs and on PineTime it works absolutely fine. Doesn't break anything. Correct touch driver is detected.

On P8 also no major issues. Tested and works fine:

  • Touch, swipes
  • Tap to wake (wow!)
  • Wake on Raise Wrist
  • Shake wake

Thanks a lot @StarGate01 for your work, my P8 has updated FW after long time. Don't know which P8 version I have, don't have internal photos, didn't looked at info page before I erased original FW.

https://twitter.com/hubmartin/status/1507463707936370696

image

hubmartin avatar Mar 25 '22 21:03 hubmartin

Great! Thanks for the shoutout :) Your watch appears to be exactly the same as my P8b (see https://github.com/InfiniTimeOrg/InfiniTime/issues/62#issuecomment-1079145003). Glad to see the firmware working.

Yea, setting up the SC7A20 to correctly detect tap events was a bit of a challenge, but I am happy it works now.

StarGate01 avatar Mar 25 '22 21:03 StarGate01

I added a new CMake configuration option (DRIVER_TOUCH), which is used to control how the CSTx17 touch driver chip is configured. On some models, this configuration is set once in the factory, and cannot be changed or even detected at runtime. I also released a new build with support for the P8a touch sensor: https://github.com/StarGate01/p8b-infinitime/releases .

StarGate01 avatar Apr 04 '22 00:04 StarGate01

I have squashed and reordered the commits, and removed the acceleration tap detection functionality, which turned out to be highly unreliable. I hope the commits are now more easily reviewable.

I also edited the PR description above, to reflect the updates to the touch driver compatibility and BLE motion service.

StarGate01 avatar Apr 06 '22 21:04 StarGate01

@Riksu9000 Thank you for the feedback!

I'll try to split the PR further into chunks, the accelerometer FIFO handling should be easy to separate for example.

Do you prefer to have fewer, larger commits per PR, or more fine-grained ones? Also, should I try to keep the PRs independent from each other, or should I stack/rebase them onto each other? The code is not always able to be cleanly separated.

StarGate01 avatar May 10 '22 18:05 StarGate01

I don't have a strong preference for the number of commits. Do whatever you think is reasonable. Keeping the PRs independent where possible should be easier and faster to review.

Riksu9000 avatar May 10 '22 18:05 Riksu9000

I have split this PR into seven new ones. I hope this makes the code significantly easier to review.

I listed all the PRs in the stating post of this PR. In the description of each PR dependencies are listed, but most are not depending on each other.

This PR now contains a big merge of all these branches. I'll leave it for historical reasons, or to see all the changes in one place.

Thanks for any help and reviews.

StarGate01 avatar May 10 '22 23:05 StarGate01

@StarGate01 Thank you very much for this extensive work on InfiniTime! These PR are well split and documented :medal_sports:

Please note that we'll probably need a bit of time to review those PRs as porting to other device is currently not our main priority.

JF002 avatar May 14 '22 09:05 JF002

@JF002 Thanks for taking the time to review. I realize that this is quite a bit of code, so I completely understand that reviewing and adjusting code will take its fair time. Any feedback is welcome, I'll do my best to integrate it into the PRs.

StarGate01 avatar May 14 '22 16:05 StarGate01