Skip to content

Bring platform layer examples #70

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
Merged
merged 14 commits into from
Mar 12, 2020
Merged

Bring platform layer examples #70

merged 14 commits into from
Mar 12, 2020

Conversation

mprse
Copy link
Contributor

@mprse mprse commented Feb 21, 2020

Bring platform layer examples +readme files.

@mprse
Copy link
Contributor Author

mprse commented Feb 24, 2020

@jamesbeyond

CI fails because BufferedSerial class is not available. I have used it instead UARTSerial which is now deprecated.

BufferedSerial was introduced over one month ago: PR ARMmbed/mbed-os#12207.

I see that CI is doing: mbed update. But I'm not sure if this command really updates mbd-os?

jamesbeyond
jamesbeyond suggested changes Feb 24, 2020
View reviewed changes
APIs_Platform/FileHandle/main.cpp Outdated
static DigitalOut led2(LED2);

// UARTSerial derives from FileHandle
static UARTSerial device(STDIO_UART_TX, STDIO_UART_RX);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the UARTSerial will be deprecated soon, can you replace it with the new one:
https://github.com/ARMmbed/mbed-os/blob/master/docs/design-documents/drivers/serial/serial.md

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, changed to BufferedSerial.

@jamesbeyond
Copy link
Contributor

I see that CI is doing: mbed update. But I'm not sure if this command really updates mbd-os?
the local version of mbed-os.lib is pointed to the latest master https://github.com/ARMmbed/mbed-os-examples-docs_only/blob/master/mbed-os.lib
mbed update should only change mbed-os when we are on release branches

@jamesbeyond
Copy link
Contributor

Hey @mprse, I think I know the problem, it is because your branch is out of date. could rebase your branch to latest master? the CI should just work

@mprse mprse force-pushed the platform_examples branch from c423bdb to ef95dfd Compare February 26, 2020 06:43
@mprse
Copy link
Contributor Author

mprse commented Feb 26, 2020

@jamesbeyond Looks like rebase helped, Thank you!

Now I got a problem with mbed_app.json. Locally it works when using mbed compile, but here we are using mbed test and this might be a problem.

@mprse mprse mentioned this pull request Mar 2, 2020
Merged
jamesbeyond
jamesbeyond approved these changes Mar 3, 2020
View reviewed changes
Copy link
Contributor

@jamesbeyond jamesbeyond left a comment

Choose a reason for hiding this comment

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

LGTM

@jamesbeyond
Copy link
Contributor

Hi @evedon could you take a look, see if you are happy about these example snippets ?

@jamesbeyond jamesbeyond requested a review from evedon March 3, 2020 16:25
@jamesbeyond
Copy link
Contributor

Maybe one thing I'd like to advise is adding a simple version of license @mprse

/*
 * Copyright (c) 2006-2020 Arm Limited and affiliates.
 * SPDX-License-Identifier: Apache-2.0
 */

@evedon
Copy link

evedon commented Mar 3, 2020 •
edited
Loading

I thought that Martin suggested a 2-line copyright header for code snippets.
Sorry @jamesbeyond did not see your previous comment!

@mprse
Copy link
Contributor Author

mprse commented Mar 4, 2020

Added missing license header.

evedon
evedon requested changes Mar 5, 2020
View reviewed changes
@mprse mprse force-pushed the platform_examples branch from f026727 to 4b1dd84 Compare March 5, 2020 19:23
@mprse
Copy link
Contributor Author

mprse commented Mar 5, 2020

Rebased on the master to resolve conflicts.

@mprse mprse force-pushed the platform_examples branch from 4b1dd84 to 6327690 Compare March 6, 2020 07:48
@jamesbeyond
Copy link
Contributor

do you happy about the changes @evedon

evedon
evedon reviewed Mar 10, 2020
View reviewed changes
APIs_Platform/ScopedRomWriteLock/README.md Outdated
@@ -0,0 +1,3 @@
## ScopedRamExecutionLock example
Copy link

Choose a reason for hiding this comment

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

Should be ScopedRomWriteLock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@mprse mprse force-pushed the platform_examples branch 2 times, most recently from 5a150ed to a6ee41e Compare March 11, 2020 18:41
@mprse
Copy link
Contributor Author

mprse commented Mar 11, 2020

Rebased on the master.

@mprse mprse force-pushed the platform_examples branch 2 times, most recently from 71c8f20 to 29b5b88 Compare March 12, 2020 10:05
@mprse mprse force-pushed the platform_examples branch from 29b5b88 to 7587f69 Compare March 12, 2020 10:08
evedon
evedon approved these changes Mar 12, 2020
View reviewed changes
Copy link

@evedon evedon 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!

@jamesbeyond jamesbeyond merged commit fad81ab into ARMmbed:master Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evedon evedon evedon approved these changes

@jamesbeyond jamesbeyond jamesbeyond approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mprse @jamesbeyond @evedon