Skip to content
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

Add basic hardware configuration options for P8 #1128

Merged
merged 2 commits into from
Jun 27, 2022

Conversation

StarGate01
Copy link
Contributor

@StarGate01 StarGate01 commented May 10, 2022

This PR has been broken out of #1050.

This enables the compile-time configuration of the LFCLK source, as well as the target hardware board pin configuration.

The WATCH_COLMI_P8 compilation option has been replaced with TARGET_DEVICE, see doc/buildAndProgram.md.

src/drivers/PinMap.h Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
doc/buildAndProgram.md Outdated Show resolved Hide resolved
@Riksu9000 Riksu9000 added this to the 1.10.0 milestone May 12, 2022
@StarGate01 StarGate01 force-pushed the p8b-base branch 2 times, most recently from efd3579 to b7beaa3 Compare May 15, 2022 21:56
@StarGate01
Copy link
Contributor Author

As discussed, I reduced the CMake configuration to TARGET_DEVICE, which sets all the required preprocessor variables for the selected device.

I rebased #1130 and #1132 to use this system as well.

src/main.cpp Outdated Show resolved Hide resolved
src/main.cpp Outdated Show resolved Hide resolved
@StarGate01
Copy link
Contributor Author

I modified the clock setup, as discussed.

Copy link
Contributor

@Riksu9000 Riksu9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks should be passed before merging.

@StarGate01 StarGate01 force-pushed the p8b-base branch 2 times, most recently from e771d8f to 87a1743 Compare May 17, 2022 19:19
@StarGate01
Copy link
Contributor Author

@Riksu9000 I figured out why I included nrfx_clock_lfclk_start previously.

The call is absolutely required, in the case that InfiniTime is isntalled via reloader-facory, coming from Wasp-OS. This is because Wasp-OS uses the internal RC oscillator, while Inifinitime switches to the external crystal osciallator if one is available.

Since there is no cold boot between the installation, there are some leftovers in the clock peripheral configuration, which confuse the NRF SDK to incorrectly assume the clock state.

I tested it, and without this call the InfiniTime recovery fails to establish a Bluetooth connection after being installed by the Wasp-OS reloader. I added the line back in c5a3d8c .

@Riksu9000
Copy link
Contributor

Am I understanding this correctly that this still only applies to devices that use the RC oscillator? If that is the case, maybe those lines only needs to be included on devices that use it.

@StarGate01
Copy link
Contributor Author

StarGate01 commented May 25, 2022

Not quite, because Wasp-OS (at least its bootloader and reloader) always use the RC oscillator for compatibility reasons. This error even happens on a genuine Pinetime when sideloading InfiniTime from Wasp-OS, I tested it.

It might be possible to circumvent this issue by draining the battery, but that is annoying imho.

@Riksu9000
Copy link
Contributor

https://wiki.pine64.org/wiki/Switching_your_PineTime_between_InfiniTime_and_Wasp-os

TBH I haven't used Wasp OS and I'm not familiar with the reloader things, however this site claims that that file is broken and instructs using some other files. @JF002 Do you know more about this #1128 (comment)?

@StarGate01
Copy link
Contributor Author

StarGate01 commented May 25, 2022

Yes, that's why I wrote my own toolchain to build reloaders that actually work and use the current version of InfiniTime, see https://github.com/StarGate01/p8b-infinitime . The reloader is the same as the one on the page you linked; it packages the pinetime-recovery-loader as payload, but uses up-to-date wasp-os and InfiniTime modules.

Although I focus on the P8a and P8b, the releases page of my repo also has versions for the Pinetime.

In any case, nrfx_clock_lfclk_start is idempotent, i.e. calling it does not disrupt anything if the clock is already running.

@JF002
Copy link
Collaborator

JF002 commented May 26, 2022

@StarGate01 Your work on porting InfiniTime to the P8 smartwatches is impressive!

As you may know, porting InfiniTime to other hardware is not on my todo list as I probably won't have the time to maintain and support it. However, I'm totally open to accept changes to support new hardware if they don't break InfiniTime on the PineTime. I would also appreciate if you would be willing to help with maintenance and support of those change. Seeing how advanced your fork for the P8 is, this will probably not be an issue :)

Do you know more about this #1128 (comment)?

Not really. The reloader was mostly built by Daniel from Waspos. I tested them (from infinitime to waspos and revert) at that time and they seemed to work fine. But I haven't done any new test or maintenance recently. Those reloader apps probably need to be updated and tested again!

src/main.cpp Show resolved Hide resolved
@StarGate01
Copy link
Contributor Author

StarGate01 commented May 26, 2022

@JF002

Your work on porting InfiniTime to the P8 smartwatches is impressive!

Thank you! I think there are quite a lot of P8s in circulation which would profit from the support, since they are cheap and available watches. Reduce e-waste and cloud dependency.

However, I'm totally open to accept changes to support new hardware if they don't break InfiniTime on the PineTime

I routinely test all my changes against a genuine PineTime in order to ensure that nothing breaks. E.g. #1145 only exists to ensure feature-parity for the PineTime concerning #1133 .

I would also appreciate if you would be willing to help with maintenance and support of those change

Of course. Just having the support upstreamed would be a massive help for me personally, since maintaining and rebasing a fork and ~10 PRs is kind of complex. Since most of my changes are hardware drivers, I don't expect much maintenance load. Ideally, all future changes would just run on the unified hardware interface.

Those reloader apps probably need to be updated

I provide a reloader-factory.zip built from the most recent code of the modules for all there watches with every release on https://github.com/StarGate01/p8b-infinitime/releases . Once the changes are upstreamed, I can work on updating the reloaders and the wiki page.

@Riksu9000
Copy link
Contributor

I realized that there should probably be some instructions on how users can identify their P8 models. I also found this issue about a third model StarGate01/p8b-infinitime#2. Is there any way to reliably determine the exact hardware a user has without experimentation, like for example the information available in the stock firmware? Any reason not to use the original MOY-*** labels to identify the devices?

@StarGate01
Copy link
Contributor Author

I would like to use the MOY* labels for indentification, if someone can help me collect enough information I'd be happy to add them to my list: https://github.com/StarGate01/p8b-infinitime/blob/master/HardwareVariants.md . However, I am unsure on what the MOY* labels actually mean - could be a reseller / software version as well (?) .

Detecting the hardware at runtime is hard, because the different chips use e.g. the same I2C address, but behave differently. There is also little to no way of detecting the touch driver reliably. In addition, it is afaik not possible to know whether an external LF crystal exists without looking at the PCB.

@StarGate01
Copy link
Contributor Author

StarGate01 commented Jun 22, 2022

@JF002 I'll set this to a draft for today - I'll convert the P8A / P8B strings into the "official" MOY ones before merging.

@StarGate01
Copy link
Contributor Author

StarGate01 commented Jun 25, 2022

I have adapted the identifiers. The watch by @ildar (StarGate01/p8b-infinitime#2), the MOY-TIN5 has been added, as well as the watch by @izzeho (StarGate01/p8b-infinitime#3, requires patch at #1200). The identifier of that watch is not known until I receive my order, so in the meantime it is called MOY-UNK.

@StarGate01 StarGate01 marked this pull request as ready for review June 25, 2022 18:45
doc/buildAndProgram.md Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
This enables the configuration of the LFCLK source,
as well as the target hardware board pin configuration.
@Riksu9000 Riksu9000 merged commit e77d47e into InfiniTimeOrg:develop Jun 27, 2022
@StarGate01 StarGate01 deleted the p8b-base branch June 27, 2022 05:34
@ildar
Copy link

ildar commented Oct 11, 2022 via email

@ildar
Copy link

ildar commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants