UnbufferedSerial: Introduce the class to replace RawSerial #11961

hugueskamba · 2019-11-27T14:46:52Z

Description

Summary of change

Deprecate RawSerial.
Introduce UnbufferedSerial to provide unbuffered I/O by implementing
with a FileHandle interface for I/O streams.
Add Greentea test for the UnbufferedSerial class.

See the comparison below.

Note:
The RawSerial app and the UnbufferedSerial app were built with LTO.
LTO for the ARM toolchain provides a RAM saving of 262 bytes and ROM saving up to 6069 bytes.
LTO for the GCC_ARM toolchain provides a RAM saving of 216 bytes and ROM saving up to 2869 bytes.

The comparison below is between the applications when they were built with LTO.

Comparing:
* Simple application which creates an instance of RawSerial (DirectSerial in retarget)
* Simple application which creates an instance of UnbufferedSerial (DirectSerial in retarget)

ARM:
Total Static RAM memory (data + bss): 0 bytes (Rawserial app: 205724, UnbufferedSerial app: 205724)
Total Flash memory (text + data): +133 bytes (Rawserial app: 43215, UnbufferedSerial app: 43348)

GCC_ARM:
Total Static RAM memory (data + bss): 0 bytes (Rawserial app: 12312, UnbufferedSerial app: 12312)

Total Flash memory (text + data): -216 bytes (Rawserial app: 58064, UnbufferedSerial app: 57848)

UnbufferedSerial with DirectSerial in retarget is more memory efficient than RawSerial with DirectSerial in retarget (with or without LTO) for GCC_ARM toolchain but less efficient for ARM toolchain.

Documentation

https://github.com/ARMmbed/mbed-os/blob/master/docs/design-documents/drivers/serial/serial.md#detailed-design--unbufferedserial

Pull request type

[] 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

[] 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

@evedon @kjbracey-arm

Release Notes

Summary of changes

Add the UnbufferedSerial class to provide unbuffered I/O access.
It is intended to be used instead of RawSerial. See documentration section for more details.

Impact of changes

Migration actions required

ciarmcom · 2019-11-27T16:00:40Z

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

kjbracey

Looks pretty good. Think you'll need one change in the retarget bit.

platform/source/mbed_retarget.cpp

drivers/source/UnbufferedSerial.cpp

mbed.h

drivers/RawSerial.h

drivers/source/UnbufferedSerial.cpp

0xc0170 · 2019-12-03T11:52:20Z

Also docs PR reference here please.

mbed-ci · 2019-12-03T12:34:08Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_build-GCC_ARM

0xc0170 · 2019-12-03T13:07:28Z

The failures look related, please review

hugueskamba · 2019-12-10T11:19:05Z

This force-push restores DirectSerial in the sys I/O retarget code to reduce memory.

hugueskamba · 2019-12-10T13:17:46Z

This force-push updates the commit message as DirectSerial has been restored.

hugueskamba · 2019-12-10T13:52:38Z

When using LTO

hugueskamba · 2019-12-10T14:17:18Z

@kjbracey-arm @bulislaw @evedon

See the comparison below.

Note:
The RawSerial app and the UnbufferedSerial app were built with LTO.
LTO for the ARM toolchain provides a RAM saving of 262 bytes and ROM saving up to 6069 bytes.
LTO for the GCC_ARM toolchain provides a RAM saving of 216 bytes and ROM saving up to 2869 bytes.

The comparison below is between the applications when they were built with LTO.

Comparing:
* Simple application which creates an instance of RawSerial (DirectSerial in retarget)
* Simple application which creates an instance of UnbufferedSerial (DirectSerial in retarget)

ARM:
Total Static RAM memory (data + bss): 0 bytes (Rawserial app: 205724, UnbufferedSerial app: 205724)
Total Flash memory (text + data): +133 bytes (Rawserial app: 43215, UnbufferedSerial app: 43348)

GCC_ARM:
Total Static RAM memory (data + bss): 0 bytes (Rawserial app: 12312, UnbufferedSerial app: 12312)

Total Flash memory (text + data): -216 bytes (Rawserial app: 58064, UnbufferedSerial app: 57848)

UnbufferedSerial with DirectSerial in retarget is more memory efficient than RawSerial with DirectSerial in retarget (with or without LTO) for GCC_ARM toolchain but less efficient for ARM toolchain.

evedon · 2019-12-10T14:48:52Z

@kjbracey-arm @bulislaw @evedon

See the comparison below.

Note:
The RawSerial app and the UnbufferedSerial app were built with LTO.
LTO for the ARM toolchain provides a RAM saving of 263 bytes and ROM saving up to 6135 bytes.
LTO for the GCC_ARM toolchain provides a RAM saving of 296 bytes and ROM saving up to 2933 bytes.

The comparison below is between the applications when they were built with LTO.
Comparing:
* Simple application which creates an instance of RawSerial (DirectSerial in retarget)
* Simple application which creates an instance of UnbufferedSerial (DirectSerial in retarget)

ARM:
Total Static RAM memory (data + bss): 0 bytes (Rawserial app: 205724, UnbufferedSerial app: 205724)
Total Flash memory (text + data): +133 bytes (Rawserial app: 43215, UnbufferedSerial app: 43348)

GCC_ARM:
Total Static RAM memory (data + bss): 0 bytes (Rawserial app: 12312, UnbufferedSerial app: 12312)

Total Flash memory (text + data): -216 bytes (Rawserial app: 58064, UnbufferedSerial app: 57848)
UnbufferedSerial with DirectSerial in retarget is more memory efficient than RawSerial

The numbers are quite good. I think the extra 133 bytes of Flash for ARM toolchain is a price worth paying for UnbufferedSerial

hugueskamba · 2019-12-10T15:43:25Z

This force-push removes the static pinmap changes from this branch. Another PR has been created to address that issue.

mbed-ci · 2019-12-10T19:20:44Z

Test run: FAILED

Summary: 2 of 4 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_unittests
jenkins-ci/mbed-os-ci_build-ARM

0xc0170 · 2019-12-11T08:31:00Z

CI restarted

mbed-ci · 2019-12-11T11:17:32Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

hugueskamba · 2019-12-11T11:39:20Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

@0xc0170
I am confused. I cannot find any failure except for this but I think the same happened on someone else's PR. What seems to be the issue?

0xc0170 · 2019-12-11T13:29:48Z

Restarted tests to confirm the failures and provide reports

* Deprecate RawSerial. * Introduce UnbufferedSerial to provide unbuffered I/O by implementing with a FileHandle interface for I/O streams. * Add Greentea test for the UnbufferedSerial class.

hugueskamba · 2019-12-12T09:05:18Z

This force-push modifIes the newly added test to take into account targets that remove all bytes from the FIFO.

adbridge · 2019-12-12T10:37:38Z

CI started

mbed-ci · 2019-12-12T13:14:40Z

Test run: SUCCESS

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

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 86fff81 to 3ad8002 Compare November 27, 2019 15:21

ciarmcom requested review from evedon, kjbracey and a team November 27, 2019 16:00

ciarmcom added the needs: review label Nov 27, 2019

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 3ad8002 to 01d72ef Compare November 29, 2019 16:08

kjbracey suggested changes Dec 2, 2019

View reviewed changes

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch 4 times, most recently from 707656f to ed45bef Compare December 2, 2019 13:55

kjbracey reviewed Dec 2, 2019

View reviewed changes

drivers/source/UnbufferedSerial.cpp Outdated Show resolved Hide resolved

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from ed45bef to a39e03b Compare December 2, 2019 14:38

kjbracey approved these changes Dec 2, 2019

View reviewed changes

evedon reviewed Dec 2, 2019

View reviewed changes

drivers/source/UnbufferedSerial.cpp Outdated Show resolved Hide resolved

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from a39e03b to 6f88b2c Compare December 3, 2019 09:29

kjbracey approved these changes Dec 3, 2019

View reviewed changes

evedon approved these changes Dec 3, 2019

View reviewed changes

0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Dec 3, 2019

0xc0170 approved these changes Dec 3, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Dec 3, 2019

0xc0170 added needs: work and removed needs: CI labels Dec 3, 2019

0xc0170 added needs: CI and removed needs: work labels Dec 10, 2019

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 63c34b5 to 77cf419 Compare December 10, 2019 11:17

hugueskamba changed the title ~~UnbufferedSerial: Replace DirectSerial and RawSerial classes~~ UnbufferedSerial: Introduce the class to replace RawSerial Dec 10, 2019

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 77cf419 to 323b81d Compare December 10, 2019 13:16

hugueskamba closed this Dec 10, 2019

hugueskamba reopened this Dec 10, 2019

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 323b81d to 142b1d2 Compare December 10, 2019 14:22

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 142b1d2 to 3885a0b Compare December 10, 2019 15:36

UnbufferedSerial: Introduce the class to replace RawSerial

7b84540

* Deprecate RawSerial. * Introduce UnbufferedSerial to provide unbuffered I/O by implementing with a FileHandle interface for I/O streams. * Add Greentea test for the UnbufferedSerial class.

hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 3885a0b to 7b84540 Compare December 12, 2019 08:44

adbridge added ready for merge and removed needs: CI labels Dec 12, 2019

adbridge merged commit d128ff4 into ARMmbed:master Dec 13, 2019

adbridge removed the ready for merge label Dec 13, 2019

hugueskamba deleted the hk-add-unbuffered-serial-class branch June 30, 2020 11:38

UnbufferedSerial: Introduce the class to replace RawSerial #11961

UnbufferedSerial: Introduce the class to replace RawSerial #11961

Uh oh!

Conversation

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

Uh oh!

Description

Summary of change

Documentation

Pull request type

Test results

Reviewers

Release Notes

Summary of changes

Impact of changes

Migration actions required

Uh oh!

ciarmcom commented Nov 27, 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!

0xc0170 commented Dec 3, 2019

Uh oh!

mbed-ci commented Dec 3, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Dec 3, 2019

Uh oh!

hugueskamba commented Dec 10, 2019

Uh oh!

hugueskamba commented Dec 10, 2019

Uh oh!

hugueskamba commented Dec 10, 2019

Uh oh!

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

Uh oh!

Uh oh!

evedon commented Dec 10, 2019

Uh oh!

hugueskamba commented Dec 10, 2019

Uh oh!

mbed-ci commented Dec 10, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Dec 11, 2019

Uh oh!

mbed-ci commented Dec 11, 2019

Test run: FAILED

Uh oh!

hugueskamba commented Dec 11, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Dec 11, 2019

Uh oh!

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

Uh oh!

Uh oh!

adbridge commented Dec 12, 2019

Uh oh!

mbed-ci commented Dec 12, 2019

Test run: SUCCESS

Uh oh!

Uh oh!

hugueskamba commented Nov 27, 2019 •

edited

Loading

hugueskamba commented Dec 10, 2019 •

edited

Loading

hugueskamba commented Dec 12, 2019 •

edited

Loading