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

Nrf52833 support #659

Closed
wants to merge 7 commits into from

Conversation

LokiMetaSmith
Copy link

adds nordic demo boards pca10059 and pca10100, also includes support for nrf52833

lawrence kincheloe and others added 7 commits June 3, 2021 16:03
Signed-off-by: lawrence kincheloe <[email protected]>
Signed-off-by: lawrence kincheloe <[email protected]>
# Conflicts:
#	boards.txt
#	variants/raytac_mdbt50q_rx/variant.h
…no.git into ada_master

# Conflicts:
#	boards.txt
#	variants/raytac_mdbt50q_rx/variant.h

Signed-off-by: lawrence kincheloe <[email protected]>
@hathach
Copy link
Member

hathach commented Jun 21, 2021

Sorry, I have been busy with other works, will review this whenever I could

Copy link
Member

@hathach hathach left a comment

Choose a reason for hiding this comment

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

I think you make accidental changes to following, therefore please revert it

  • submodule TinyUSB_Arduino
  • submodule nRFcrypto
  • changes to cpb nrf52840

I will review it more once #658 is merged, since it seems to have some conflict in platform.txt

Comment on lines +73 to +78
#define PIN_A1 (15)
#define PIN_A2 (16)
#define PIN_A3 (17)
#define PIN_A4 (18)
#define PIN_A5 (19)
#define PIN_A6 (20)
Copy link
Member

Choose a reason for hiding this comment

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

are you making these changes accidentally.

Copy link
Author

Choose a reason for hiding this comment

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

I might have pulled those from another commit. I didn't intend to change those.

Copy link
Member

@hathach hathach left a comment

Choose a reason for hiding this comment

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

Most of the modification looks good, though in additional to previous review, please

  • clarify the Opossum Pouch and its vid/pid
  • change generic board to pca10100
  • remove the pca10059 (usb dongle) since the board requires soldering swd and flashing bootloader before actually using with this repo. Which is not really arduino friendly and could raise lots of support ticket from user.



# ----------------------------------
# Generic NRF52833
Copy link
Member

Choose a reason for hiding this comment

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

generic board is not accepted, this should be changed to pca10100 if that is what you mean.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that's the intent. I was reworking another commit and missed the documentation name change.

Comment on lines +425 to +426
op1.build.vid=0x239A
op1.build.pid=0x8029
Copy link
Member

Choose a reason for hiding this comment

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

can you tell us more about this board. Also each board must has its own dedicated VID/PID and cannot re-use PID from another especially one is selling as commercial product.

Copy link
Author

Choose a reason for hiding this comment

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

That's a mistake, Opossum Pouch is the board that I'm developing. I've got it defined on a private fork.

* code flash 0x00000000
*/

FLASH (rx) : ORIGIN = 0x26000, LENGTH = 0xED000 - 0x26000
Copy link

Choose a reason for hiding this comment

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

I think this should be

FLASH (rx)     : ORIGIN = 0x26000, LENGTH = 0x6D000 - 0x26000

0xED000 is too long

@jpconstantineau
Copy link
Contributor

This PR would resolve #696

#696 was created in order to enable support of other nrf52833 boards in new tracking repo...

@LokiMetaSmith I am planning to have a fork that will include many non-adafruit boards, including the 840 dongle. I have a "Community nRF52 Board Support package" but with all the changes that I made in it and those thatoccurred earlier this year in upstream, it's a bit of a mess especially that it's not set to track upstream. My new tracking fork isn't quite ready yet as I still have to get CI updated.

@hathach
Copy link
Member

hathach commented Jun 5, 2024

52833 is supported by #739. Thank you for your patient

@hathach hathach closed this Jun 5, 2024
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.

4 participants