mbed_retarget: Add a minimal console implementation to provide basic functionalities #11796

hugueskamba · 2019-11-01T12:54:24Z

Description (required)

The retarget code allocates an array of FileHandle* for console and file
handling (filehandles). A tiny target only needs a console (putc/getc).
There is no need for file handling.

The POSIX layer and the array of FileHandle* is not required for small
targets that only need a console ; this code is optionally compiled
out if the configuration parameter platform.stdio-minimal-console-only is
set to "true".

Summary of change (What the change is for and why)

Add a minimal console implementation in mbed_retarget to provide basic functionalities.

Documentation (Details of any document updates required)

#11634

Pull request type (required)

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

Test results (required)

[] 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 (optional)

@kjbracey-arm

Release Notes (required for feature/major PRs)

Summary of changes

Add a minimal console implementation in mbed_retarget to provide basic console functionalities.
A new configuration parameter platform.stdio-minimal-console-only has been added to select it.

Impact of changes

Smaller memory footprint for applications that only require basic console functionalities.

Migration actions required

N/A

ciarmcom · 2019-11-01T14:00:31Z

@hugueskamba, thank you for your changes.
@kjbracey-arm @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

evedon

Great work.
Can you add in the "Release Notes" section of the PR that there is a new configuration option platform.stdio-minimal-console-only

hugueskamba · 2019-11-01T15:27:42Z

This force-push sets the default value of the platform.stdio-minimal-console-only parameter to false as existing applications and tests may rely on the use of the unbuffered serial class and/or APIs that are removed when the parameter is set to true.

kjbracey

Looks like it should be a good saving. Can refine it a bit.

platform/mbed_lib.json

platform/mbed_retarget.h

platform/source/mbed_retarget.cpp

hugueskamba · 2019-11-06T11:21:34Z

The two force-pushes (this and this) addresses the first round of comments by @kjbracey-arm

platform/source/mbed_retarget.cpp

platform/mbed_retarget.h

platform/source/mbed_retarget.cpp

platform/Stream.h

hugueskamba · 2019-11-08T11:31:14Z

This force-push removes fdopen.

kjbracey · 2019-11-11T11:10:34Z

The actual MinimalConsole class has vanished, so check for remaining references and remove.

platform/Stream.h

platform/source/mbed_retarget.cpp

hugueskamba · 2019-11-11T13:06:53Z

This force-push addresses the latest round of comments by @kjbracey-arm

kjbracey

Few more adjustments... And see my comment about removing references to "class" and "MinimalConsole" - still there in commit message and GitHub description.

platform/mbed_retarget.h

platform/source/mbed_retarget.cpp

hugueskamba · 2019-11-11T15:05:46Z

This force-push addresses the latest round of comments by @kjbracey-arm.

…functionalities The retarget code allocates an array of FileHandle* for console and file handling (filehandles). A tiny target only needs a console (putc/getc). There is no need for file handling. The POSIX layer and the array of FileHandle* is not required for small targets that only need a console ; this code is optionally compiled out if the configuration parameter platform.stdio-minimal-console-only is set to `"true"`.

0xc0170 · 2019-11-12T09:20:51Z

I'll review shortly, started CI meanwhile

bulislaw

That looks good, but we need to document this option in our handbook. Why/when would you use it, what difference does it make and what are the consequences.

JojoS62 · 2019-11-12T12:25:07Z

as you are currently working on fdopen:
I have the problem that an class derives from Stream and implements putc. This works when it is compiled with gcc under windows, but same gcc version under MacOS produces mbed_die.
I've traced the problem and it happens in the constuctor of Stream, calling fdopen. This fdopen returns NULL, no valid filed handle. In my understanding, there must be an open handle fd that fdopen accepts. But I cannot find a open function that is called before. Is it possible that the fdopen function has different behaviour or am I overseeing a call of fopen somewhere before?

kjbracey · 2019-11-12T12:34:03Z

I have the problem that an class derives from Stream and implements putc. This works when it is compiled with gcc under windows, but same gcc version under MacOS produces mbed_die.

The fdopen from Stream is an mbed-specific overload taking FileHandle * instead of int, implemented in mbed_retarget.cpp It uses the standard fdopen(int) internally. You should be able to single-step through enough to see whether the problem is in the Mbed OS half or the library fdopen.

Please create an issue if you want to continue the discussion - we shouldn't derail discussion on this PR.

mbed-ci · 2019-11-12T12:35:16Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

hugueskamba · 2019-11-13T07:45:40Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

@0xc0170 should the test just be re-started?

0xc0170 · 2019-11-13T13:30:14Z

Test restarted

0xc0170 · 2019-11-13T14:58:29Z

All green now!

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch from c204bf1 to a37de1c Compare November 1, 2019 13:04

ciarmcom requested review from kjbracey and a team November 1, 2019 14:00

ciarmcom added the needs: review label Nov 1, 2019

evedon approved these changes Nov 1, 2019

View reviewed changes

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch from a37de1c to 94a042d Compare November 1, 2019 15:24

kjbracey suggested changes Nov 5, 2019

View reviewed changes

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch 2 times, most recently from af99484 to 8eec656 Compare November 6, 2019 11:20

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch from 8eec656 to 1180b3c Compare November 7, 2019 08:15

kjbracey reviewed Nov 7, 2019

View reviewed changes

platform/source/mbed_retarget.cpp Outdated Show resolved Hide resolved

platform/source/mbed_retarget.cpp Outdated Show resolved Hide resolved

platform/source/mbed_retarget.cpp Outdated Show resolved Hide resolved

platform/mbed_retarget.h Outdated Show resolved Hide resolved

0xc0170 added needs: work release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed needs: review labels Nov 7, 2019

kjbracey reviewed Nov 7, 2019

View reviewed changes

platform/source/mbed_retarget.cpp Outdated Show resolved Hide resolved

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch 6 times, most recently from 3bbd53d to 6e32242 Compare November 8, 2019 11:05

kjbracey reviewed Nov 8, 2019

View reviewed changes

platform/Stream.h Show resolved Hide resolved

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch 2 times, most recently from d00c27c to 584fb82 Compare November 8, 2019 11:30

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch from 584fb82 to af1ea58 Compare November 8, 2019 12:06

kjbracey suggested changes Nov 11, 2019

View reviewed changes

platform/Stream.h Outdated Show resolved Hide resolved

platform/source/mbed_retarget.cpp Outdated Show resolved Hide resolved

platform/source/mbed_retarget.cpp Outdated Show resolved Hide resolved

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch from e986995 to d8dd686 Compare November 11, 2019 13:05

kjbracey suggested changes Nov 11, 2019

View reviewed changes

hugueskamba changed the title ~~mbed_retarget: Add an internal class to provide basic console functionality~~ mbed_retarget: Add a minimal console implementation to provide basic functionalities Nov 11, 2019

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch from d8dd686 to 2d21e53 Compare November 11, 2019 15:04

hugueskamba force-pushed the hk-add-minimal-console-to-retarget branch from 2d21e53 to 051991f Compare November 11, 2019 15:14

kjbracey approved these changes Nov 12, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Nov 12, 2019

0xc0170 approved these changes Nov 12, 2019

View reviewed changes

bulislaw approved these changes Nov 12, 2019

View reviewed changes

0xc0170 removed the needs: CI label Nov 13, 2019

0xc0170 merged commit eab1c2e into ARMmbed:master Nov 13, 2019

hugueskamba deleted the hk-add-minimal-console-to-retarget branch November 13, 2019 16:14

0xc0170 added release-version: 5.15.0-rc1 and removed release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Nov 19, 2019

This was referenced Nov 21, 2019

UnbufferedSerial: Replace DirectSerial, RawSerial classes hugueskamba/mbed-os#5

Closed

Minimal Console: Fix syntax errors #11947

Merged

Minimal Console: Fix compilation error #11959

Merged

mbed_retarget: Add a minimal console implementation to provide basic functionalities #11796

mbed_retarget: Add a minimal console implementation to provide basic functionalities #11796

Uh oh!

Conversation

hugueskamba commented Nov 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (required)

Summary of change (What the change is for and why)

Documentation (Details of any document updates required)

#11634

Pull request type (required)

Test results (required)

Reviewers (optional)

Release Notes (required for feature/major PRs)

Summary of changes

Impact of changes

Migration actions required

Uh oh!

ciarmcom commented Nov 1, 2019

Uh oh!

evedon left a comment

Choose a reason for hiding this comment

Uh oh!

hugueskamba commented Nov 1, 2019

Uh oh!

kjbracey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugueskamba commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugueskamba commented Nov 8, 2019

Uh oh!

kjbracey commented Nov 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugueskamba commented Nov 11, 2019

Uh oh!

kjbracey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugueskamba commented Nov 11, 2019

Uh oh!

0xc0170 commented Nov 12, 2019

Uh oh!

bulislaw left a comment

Choose a reason for hiding this comment

Uh oh!

JojoS62 commented Nov 12, 2019

Uh oh!

kjbracey commented Nov 12, 2019

Uh oh!

mbed-ci commented Nov 12, 2019

Test run: FAILED

Uh oh!

hugueskamba commented Nov 1, 2019 •

edited

Loading

hugueskamba commented Nov 6, 2019 •

edited

Loading