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.

Sign up for GitHub

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

Jump to bottom

boards: add pyboard v1.1 support #11080

Merged
merged 2 commits into from
Mar 13, 2019
Merged

boards: add pyboard v1.1 support #11080

merged 2 commits into from
Mar 13, 2019

Conversation

kaspar030
Copy link
Contributor

From the docs:

Overview

This port adds basic support for the MicroPython pyboard.

Currently the support is pretty minimal, and you might miss many features that
the original firmware offers. E.g., the board won't show up via USB as either
serial or mass storage device.

In order to get serial output, connect an USB/serial adapter to USART1 RX/TX
(Pins X9 / X10).

How to flash

put board into DFU mode:

  • connect board via USB
  • connect 3.3v to DFU pin (See official docs)
  • press reset button

Flash as usual:

$ BOARD=pyboard make flash"
  • unconnect 3.3v/DFU pin
  • reset again

Testing procedure

Run the usual tests on a pyboard.

Issues/PRs references

None.

@kaspar030 kaspar030 added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: boards Area: Board ports labels Feb 28, 2019
@emmanuelsearch
Copy link
Member

@kaspar030 nice!! Comments so far:

  • I confirm I could build, flash and run
  • ran tests/leds but the leds don't blink...
  • to term I had to specify PORT=/dev/ttyUSB0

@kaspar030
Copy link
Contributor Author

  • ran tests/leds but the leds don't blink...

Fixed, led was not set up and its macro had the wrong pin number.

  • to term I had to specify PORT=/dev/ttyUSB0

fixed.

@kaspar030
Copy link
Contributor Author

Now also all issues pointed out by Murdock are fixed. @emmanuelsearch could you take another look?

aabadie
aabadie requested changes Mar 5, 2019
View reviewed changes
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Some minor nits, otherwise it's good :)

boards/pyboard/Makefile.features Outdated
@@ -0,0 +1,13 @@
# Put defined MCU peripherals here (in alphabetical order)
#FEATURES_PROVIDED += periph_dma
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no configuration provided for DMA, so I think this should be removed. Same for PWM.

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

boards/pyboard/board.c
@@ -0,0 +1,32 @@
/*
* Copyright (C) 2019 Inria
Copy link
Contributor

Choose a reason for hiding this comment

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

Inria has no copyright on this file.

Copy link
Member

@emmanuelsearch emmanuelsearch Mar 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

actually @kaspar030 IIRC you're doing this work paid by Inria + FU collaboration so should the copyright not be instead Copyright (C) 2019 Inria and Copyright (C) 2019 FU Berlin where copyright is applicable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, make sense.

boards/pyboard/include/board.h Outdated
* @file
* @brief Board specific definitions for the pyboard board
*
* @author Alexandre Abadie <alexandre.abadie@inria.fr>
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, but you can remove me (and Inria) from the list of authors and copyrights.

Copy link
Member

Choose a reason for hiding this comment

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

(same comment as above)

boards/pyboard/include/periph_conf.h Outdated
@@ -0,0 +1,187 @@
/*
* Copyright (C) 2017 Inria
Copy link
Contributor

Choose a reason for hiding this comment

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

Invalid copyright

@emmanuelsearch
Copy link
Member

works like a charm now!

@emmanuelsearch
Copy link
Member

@kaspar030 can you address the minor comments of @aabadie and squash?

@kaspar030
Copy link
Contributor Author

  • adressed comments, squashed

@aabadie I've re-used a lot of the files that you created, thus I kept the authorship. It was impressively easy to get this board working, with all the stm infrastructure in place! But ok, you're out again, so complaints go to me first. 😉

aabadie
aabadie reviewed Mar 5, 2019
View reviewed changes
boards/pyboard/doc.txt Outdated
- press reset button

## Flash as usual:
$ BOARD=pyboard make flash"
Copy link
Contributor

Choose a reason for hiding this comment

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

one last thing: I think this line won't render very well in the generated documentation. There should be a blank line before.
Also note that there's an extra " at the end of the line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for actually looking at the docs. fixed & amended.

aabadie
aabadie approved these changes Mar 6, 2019
View reviewed changes
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Looks good to me now.

Un-tested ACK. I let @emmanuelsearch give a last try and merge.

@emmanuelsearch
Copy link
Member

retested, works, let's go.

@emmanuelsearch emmanuelsearch added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Mar 13, 2019
@emmanuelsearch emmanuelsearch merged commit 8969fd3 into RIOT-OS:master Mar 13, 2019
@cladmi
Copy link
Contributor

cladmi commented Mar 14, 2019

Just found out about the dfu.inc.mk while rebasing #8838 it would have been good to split it in another pull request or have an issue for updating the boards already using dfu-util.

@aabadie
Copy link
Contributor

aabadie commented Mar 14, 2019

it would have been good to split it in another pull request or have an issue for updating the boards already using dfu-util

I thought the same when reviewing but I didn't want to add more delay on this when I ACK'ed this PR. I'll update the related boards (nz32-sc151, bluepill, spark-core) and open a PR tomorrow.

@aabadie aabadie mentioned this pull request Mar 15, 2019
Merged
@danpetry danpetry added this to the Release 2019.04 milestone Mar 26, 2019
@kaspar030 kaspar030 deleted the add_pyboard branch July 5, 2019 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabadie aabadie aabadie approved these changes

@emmanuelsearch emmanuelsearch Awaiting requested review from emmanuelsearch

Assignees
No one assigned
Labels
Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2019.04
Development

Successfully merging this pull request may close these issues.
5 participants
@kaspar030 @emmanuelsearch @cladmi @aabadie @danpetry