search

zephyrproject-rtos / gsoc-2022-arduino-core

Arduino Core Zephyr Module (GSoC 2022 Project)
Apache License 2.0
44 stars 11 forks source link

Add nrf52840dk #32

Closed szczys closed 1 year ago

szczys commented 2 years ago

This PR adds overlay and pinmap for Nordic and NXP boards that have Arduino headers.

I used the following technique for mapping these headers:

This is based on #31 which hasn't yet been merged. There is actually only one new commit in this PR: https://github.com/zephyrproject-rtos/gsoc-2022-arduino-core/pull/32/commits/d0697cacc471bc4ef76944fe1fb49069f34a9466

DhruvaG2000 commented 2 years ago

I think you need to merge the dev branch onto this branch rather than rebase... As of now, there are repeat commits from dev in this branch making the changed files look more than there are

szczys commented 2 years ago

@DhruvaG2000 Sorry about that, I've cleaned up the PR.

DhruvaG2000 commented 2 years ago

I don't really have the hardware to test this PR, but if it builds and works on your end I don't see any issues. Maybe if we could add the i2c nodes as well here it would be great for when the I2C PR finally merges

szczys commented 1 year ago

@DhruvaG2000 will you please review and approve this PR? Notes below, thanks!

Add nrf52840dk (this PR)

I have added i2c support and tested the nrf52840dk_nrf52840 and found it is working. This PR now represents all changes necessary to add that board as a variant.

New branch: Add mimxrt1060_evkb (not in this PR)

The mimxrt1060_evkb is not working in tests (scanning for i2c address returns a device on every address). The changes to add that board have been moved to a different branch so that this PR can be merged.

szczys commented 1 year ago

@DhruvaG2000 thanks for catching the checkpatch issues. I think I've fixed them, however I do still get 3 warnings. Do you know if these are problems that need fixing?

#2: 
new file mode 100644

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#40: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:1:
+/*

WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead
#43: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:4:
+ * SPDX-License-Identifier: Apache-2.0
DhruvaG2000 commented 1 year ago

Imo we can safely ignore the SPDX warnings, because visually it LGTM. Code formatting is mostly important and that seems to be fixed, I'll try and build this again on my machine tomorrow just to be sure before approving and then I think we can merge this.

szczys commented 1 year ago

Ah yes, I know that Zephyr doesn't allow // comments. Thanks for catching that!

DhruvaG2000 commented 1 year ago

@szczys Thank you so much for this contribution! A final nit, please squash the redundant "update" commits.

szczys commented 1 year ago

Okay, I have squashed and also re-targeted this PR for main.