Add initial P8 a/b smartwatch support #1050
Conversation
StarGate01
force-pushed
the
p8b
branch
2 times, most recently
from
ce83da6 to
402ebae
Compare
|
@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 Thanks! Let me know how the testing goes, especially if you have a P8a watch (which I can't test on at the moment). |
|
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:
Thanks a lot @StarGate01 for your work, my P8 has updated FW after long time. https://twitter.com/hubmartin/status/1507463707936370696
|
|
Great! Thanks for the shoutout :) Your watch appears to be exactly the same as my P8b (see #62 (comment)). 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
force-pushed
the
p8b
branch
3 times, most recently
from
d5f85c9 to
4cdc1a7
Compare
StarGate01
changed the title
|
I added a new CMake configuration option ( |
StarGate01
force-pushed
the
p8b
branch
2 times, most recently
from
0b86558 to
8b872ea
Compare
|
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
force-pushed
the
p8b
branch
2 times, most recently
from
c1666ce to
c4fc866
Compare
There was a problem hiding this comment.
Thank you for your work on porting InfiniTIme to other devices, however I think this PR is still too big even with the two other PRs. It would be a good idea to chop it down further to a PR for each driver for example. Also please follow our coding conventions and avoid creating unnecessary changes for a better chance of this being merged.
|
@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. |
|
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. |
StarGate01
force-pushed
the
p8b
branch
2 times, most recently
from
e3b944a to
63700ad
Compare
This function sends a tap down and tap up event to LVGL, despite being called only once.
The P8 smartwatches use the CST716 or CST816S chips in various modes. The device ID of these chips cannot be used for runtime detection, because it does not give the hardware revision / firmware configuration.
The AccelerationSensor base class provides a unified interface to access both the BMA421 as well as the SC7A20 chip.
The FIFO buffer is used as well
The DRIVER_ACC configuration variable can be used to select the acceleration sensor driver to be used.
The motion BLE service now broadcasts the entire FIFO buffer, in order to expose a higher frequency measurement. A high enough MTU has to be negotiated to fit all max. 192 bytes. The format is backwards-compatible.
This removes the "ISO C99 requires whitespace after the macro name" warning
|
Hello again, pals. |
|
I am planning to rebase my PRs on the most recent Infinitime release quite soon, I recently got a P8 watch again. Most of the yet unmerged PRs were held up by some discussions about implementation details, which should be able to be resolved eventually. |
Since this PR was too large, it was split smaller, separate PRs. Same-level entries can be merged independent from each other.
#1133 also requires #1145 for the BMA421.
PRs / WIPs in related projects:
This PR extends the existing P8 smartwatch support (eg. from e614af1) to support models of the P8 series: P8a and P8b. This is a feature desired by the community (see #62).
The P8 smartwatches and the Pinetime have a few small differences:
SC7A20acceleration sensor, some have aBMA421(like the Pinetime)CST716touch screen sensor, some have aCST816S(like the Pinetime)I added support for the
SC7A20sensor 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
CST716does 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. TheCST816Sis 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
CSTX16touch 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 theCST816Sas well as theCST716are configured in reporting mode. TheCST816Sis able to switch at runtime to this mode, which is what the code does. TheCST716can only be configured once in the factory. In my P8b it came in reporting mode, however there are P8a variants with aCST716stuck in gesture mode. The touch sensor mode can be specified by setting the CMake variableDRIVER_TOUCH.The Nimble Bluetooth stack has been slightly modified to be compatible with the LFRC clock source calibration (see apache/mynewt-nimble#1207), as well as the power-conserving
BLE_LL_RFMGMT_ENABLE_TIMENimble 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 theRCsource 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) replacesWATCH_COLMI_P8to 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: InfiniTimeOrg/pinetime-mcuboot-bootloader#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.