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 support for Makerdiary nRF52840-MDK #11846

Merged
merged 2 commits into from
Nov 21, 2019

Conversation

manchoz
Copy link
Contributor

@manchoz manchoz commented Nov 12, 2019

Description

Add support for MakerDiary nRF52840-MDK.

Summary of change

Added target MAKERDIARY_NRF52840_MDK inheriting from MCU_NRF52840.

ITM support has been disabled because MakerDiary nRF52840-MDK uses TRACEDATA0 pin for on-board User Button.

Pull request type (required)

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@ciarmcom
Copy link
Member

@manchoz, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Can you also attach test logs as part of the new target addition PR

PinDef(0 , 31),

PinDef(1 , 0), //P1_1 = 32...
PinDef(1 , 1),
Copy link
Contributor

Choose a reason for hiding this comment

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

can you fix alignment in this file (line 93 to 107)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logs will follow because I need to move to a non MacOS machine in order to get the USB-related tests to pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please, find both full log and specific USB tests logs - run with various system adjustments - at following gist

https://gist.github.com/manchoz/de94b521c3edf0a3f771d27f70ed8b23

@0xc0170 0xc0170 self-requested a review November 13, 2019 17:18
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 13, 2019

I reviewed the logs, there are some failures - in usb - what are these? any known issues?

@manchoz
Copy link
Contributor Author

manchoz commented Nov 13, 2019

I reviewed the logs, there are some failures - in usb - what are these? any known issues?

Although I have followed all the indications for the configuration of files/devices permissions and udev, I'm having a lot of issues trying to get both GNU/Linux (Arch Linux w/ Gnome 3.34.1) and MacOS Catalina behaving properly during USB tests, because both interfere a lot with the USB operations of mbed test. (And due to Catalina complaining with "DAPLink disk non ejected properly" at every test suite the tests are running at least 2x slower)

I have performed several test sessions and the results for USB test are often different depending on the state of the OS. I'm currently trying to setup a barebone GNU/Linux testing machine in order to double check the test results.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2019

Thanks for the feedback. You can report these to our forum or as an issue (if it is being technical), tests should be reproducible by anyone.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2019

@maciejbocianski usb tests - have you seen these errors or someone else would know?

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 15, 2019

Ci started

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@maciejbocianski
Copy link
Contributor

@fkjagodzinski

@maciejbocianski
Copy link
Contributor

@maciejbocianski usb tests - have you seen these errors or someone else would know?

@0xc0170 Checked NRF52840_DK and it looks it's only linux problem - can reproduce some (on windows works fine).
@manchoz Can you confirm that you applied all steps from https://github.com/ARMmbed/mbed-os/blob/master/TESTS/usb_device/README.md

@manchoz
Copy link
Contributor Author

manchoz commented Nov 21, 2019

@maciejbocianski yes, every step done.

I have same results for USB tests on:

  • MacBook Pro 13 2017 w/ MacOS Catalina
  • Lenovo Thinkpad X220 w/ Arch Linux (w/ and w/o sudo)
  • FriendlyElec Nano Pi Neo2 w/ Armbian Buster (w/ and w/o sudo)

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 21, 2019

I accidentally started another CI run, will complete soon

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 21, 2019

@manchoz I believe these issues should be reported via new issues.

@mbed-ci
Copy link

mbed-ci commented Nov 21, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 removed the needs: CI label Nov 21, 2019
@0xc0170 0xc0170 merged commit d0d3462 into ARMmbed:master Nov 21, 2019
@manchoz
Copy link
Contributor Author

manchoz commented Nov 21, 2019

@0xc0170 a different on from #11912?

@manchoz
Copy link
Contributor Author

manchoz commented Nov 21, 2019

@0xc0170 thank you for merging.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 21, 2019

@0xc0170 a different on from #11912?

I see, that should be sufficient.

@manchoz manchoz deleted the manchoz_makerdiary-nrf52840-mdk branch May 25, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants