Skip to content

Conversation

@hugueskamba
Copy link
Collaborator

@hugueskamba hugueskamba commented Nov 27, 2019

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

@hugueskamba hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 86fff81 to 3ad8002 Compare November 27, 2019 15:21
@ciarmcom ciarmcom requested review from a team, evedon and kjbracey November 27, 2019 16:00
@ciarmcom
Copy link
Member

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

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

@kjbracey kjbracey left a comment

Choose a reason for hiding this comment

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

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

@hugueskamba hugueskamba force-pushed the hk-add-unbuffered-serial-class branch 4 times, most recently from 707656f to ed45bef Compare December 2, 2019 13:55
@hugueskamba hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from ed45bef to a39e03b Compare December 2, 2019 14:38
@hugueskamba hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from a39e03b to 6f88b2c Compare December 3, 2019 09:29
@0xc0170 0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Dec 3, 2019
@0xc0170
Copy link
Contributor

0xc0170 commented Dec 3, 2019

Also docs PR reference here please.

@mbed-ci
Copy link

mbed-ci commented Dec 3, 2019

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
Copy link
Contributor

0xc0170 commented Dec 3, 2019

The failures look related, please review

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

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

@hugueskamba hugueskamba changed the title UnbufferedSerial: Replace DirectSerial and RawSerial classes UnbufferedSerial: Introduce the class to replace RawSerial Dec 10, 2019
@hugueskamba hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 77cf419 to 323b81d Compare December 10, 2019 13:16
@hugueskamba
Copy link
Collaborator Author

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

@hugueskamba
Copy link
Collaborator Author

When using LTO

@hugueskamba hugueskamba reopened this Dec 10, 2019
@hugueskamba
Copy link
Collaborator Author

hugueskamba commented Dec 10, 2019

@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.

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

evedon commented Dec 10, 2019

@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 hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 142b1d2 to 3885a0b Compare December 10, 2019 15:36
@hugueskamba
Copy link
Collaborator Author

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

@mbed-ci
Copy link

mbed-ci commented Dec 10, 2019

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
Copy link
Contributor

0xc0170 commented Dec 11, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Dec 11, 2019

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
Copy link
Collaborator Author

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
Copy link
Contributor

0xc0170 commented Dec 11, 2019

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 hugueskamba force-pushed the hk-add-unbuffered-serial-class branch from 3885a0b to 7b84540 Compare December 12, 2019 08:44
@hugueskamba
Copy link
Collaborator Author

hugueskamba commented Dec 12, 2019

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

@adbridge
Copy link
Contributor

CI started

@mbed-ci
Copy link

mbed-ci commented Dec 12, 2019

Test run: SUCCESS

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

@adbridge adbridge merged commit d128ff4 into ARMmbed:master Dec 13, 2019
@hugueskamba hugueskamba deleted the hk-add-unbuffered-serial-class branch June 30, 2020 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants