Skip to content

Retarget - stdio serial lazy initialization #145

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.

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

Merged
merged 3 commits into from
Jan 29, 2016

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Jan 22, 2016

We provide mbed::get_serial_stdio() to get a reference for stdio Serial
object. We should discourage to use Serial pc(USBTX, USBRX)

We should clean our tests/examples, to use this new function for stdio Serial operations like baud(), format() or any other.

Should fix #140

@marcuschangarm @bogdanm @pan- @bremoran

We provide mbed::get_serial_stdio() to get a reference for stdio Serial
object. We should discourage to use Serial pc(USBTX, USBRX)
@bogdanm
Copy link
Contributor

bogdanm commented Jan 22, 2016

Could you also please update README.md with more information about this change?

@andresag01
Copy link

Power consumption of ble-examples/BLE_EddystoneService seems to be fine when using this change and the yotta config option:

"minar": {
    "no_runtime_warnings": true
}

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 25, 2016

I added section for stdio retargeting

@bogdanm
Copy link
Contributor

bogdanm commented Jan 28, 2016

+1

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 29, 2016

Just tested just in case this can break anything in this repo

+-----------------+---------------+------------------------------------+--------+--------------------+-------------+
| target          | platform_name | test                               | result | elapsed_time (sec) | copy_method |
+-----------------+---------------+------------------------------------+--------+--------------------+-------------+
| frdm-k64f-armcc | K64F          | mbed-drivers-test-basic            | OK     | 2.35               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-blinky           | OK     | 5.44               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-cpp              | OK     | 2.4                | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-cstring          | OK     | 2.94               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-detect           | OK     | 1.44               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-dev_null         | OK     | 4.51               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-div              | OK     | 2.42               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-echo             | OK     | 5.29               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-heap_and_stack   | OK     | 2.41               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-hello            | OK     | 1.39               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-rtc              | OK     | 5.57               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-serial_interrupt | OK     | 5.13               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-sleep_timeout    | OK     | 3.4                | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-stdio            | OK     | 1.75               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-stl              | OK     | 2.95               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-ticker           | OK     | 12.4               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-ticker_2         | OK     | 12.41              | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-ticker_3         | OK     | 12.39              | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-time_us          | OK     | 12.4               | shell       |
| frdm-k64f-armcc | K64F          | mbed-drivers-test-timeout          | OK     | 12.52              | shell       |
+-----------------+---------------+------------------------------------+--------+--------------------+-------------+

Similar for gcc

0xc0170 added a commit that referenced this pull request Jan 29, 2016
Retarget - stdio serial lazy initialization
@0xc0170 0xc0170 merged commit 4d98c79 into ARMmbed:master Jan 29, 2016
@BlackstoneEngineering
Copy link

Why was this accepted? This breaks ALL existing code examples!

@bogdanm
Copy link
Contributor

bogdanm commented Feb 18, 2016

The code examples were also updated. If you know of one that we miss, please let us know and we'll update it.

@marcuschangarm
Copy link
Contributor

I thought this got a version bump when it was merged?

@bogdanm
Copy link
Contributor

bogdanm commented Feb 18, 2016

It did : 3f6fb0d

@marcuschangarm
Copy link
Contributor

@BlackstoneEngineering how are you including mbed-drivers in your examples? with ~ or ^?

@BlackstoneEngineering
Copy link

@bogdanm you are not accounting for all the code examples on my computer, in my workspace, and in other areas that you no longer have control over. It is completely un-acceptable to do something like this which affects users without properly using semantic versioning. This breaks user code, it should be a major revision number bump.

@bogdanm
Copy link
Contributor

bogdanm commented Feb 18, 2016

Which would be the case if we were using major versions on mbed-drivers, which we aren't (and never have been). mbed-drivers is still at 0.x.y. This is why we bumped the minor when we did this change, not the major.

@BlackstoneEngineering
Copy link

@marcuschangarm

"dependencies": {
    "mbed-hal-frdm-k64f":"sg-/mbed-hal-frdm-k64f#crc-uuid",
    "mbed-hal-k64f":"sg-/mbed-hal-k64f#add-crc",
    "mbed-client": "^1.0.0"

@BlackstoneEngineering
Copy link

@bogdanm This is unacceptable, you are effectively magically breaking things for anyone trying to use our code base. Please start using semantic versioning properly and quit hiding in major version 0. Breaking users code is completely unacceptable.

By using major version 0 you are saying that I shouldnt use your module, which means I shouldnt use mbed-drivers, which means I shouldnt be using mbed OS. If this is the case then no one should use it.

@bogdanm
Copy link
Contributor

bogdanm commented Feb 18, 2016

We will start to use major versions on mbed-drivers when we consider it to be stable, which is completely within the specs of semver, according to semver.org:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

@marcuschangarm
Copy link
Contributor

@bogdanm @BlackstoneEngineering

Here is the problem:

mbed-client->mbed-client-mbed-os:

  "dependencies": {
  "mbed-drivers": "<1.0.0"

https://github.com/ARMmbed/mbed-client-mbed-os/blob/master/module.json#L10

@BlackstoneEngineering
Copy link

@bogdanm , please quit playing semantic games, you are hiding behind the 'definition' of the spec rather than the 'spirit' of the spec. The purpose of Semver is to provide stability on an unstable landscape. By hiding behind major version 0 and not incrimenting it you are depriving users of a stable system. Without stability no one should use our product.

Either start using semver properly (ie get out of major version 0) and provide stability, or accept that NO ONE will use this product untill it hits major version 1.

@BlackstoneEngineering
Copy link

@bogdanm By breaking my code, in my workspace, on my computer, you have lost my trust, I will no longer develop anything using your code until you use semantic versioning properly and give me some sort of reproducible stability.

@0xc0170 0xc0170 deleted the dev_stdio branch February 18, 2016 18:02
@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 18, 2016

develop anything using your code

@BlackstoneEngineering It's our code.

Use semver even in the examples if you depend on mbed-drivers please. We are doing our best to make changes backwards compatible. This serial stdio however was broken before, there were problems with it in HAL, in the common code plus with toolchains (diff behavior for stdio write)..

@marcuschangarm Thanks for pinpointing the json module file, it should be updated

@BlackstoneEngineering
Copy link

@0xc0170 you are missing the point, because we are not getting out of major version 0 there is zero reproducability in the examples. If i compile an example today I expect it to still compile in a week, under the current system of major version 0 I have no guarantee or even an expectation that my example will still work. This makes it impossible to do developer enablement, workshops, demos, or anything else that uses major version 0 modules.

@tabarr
Copy link

tabarr commented Feb 18, 2016

@BlackstoneEngineering, as an "End User" I have to say it is pretty hard for me to see mbedos as anything but a "Work in Progress" because the majority of the development is being done a on single processor. If mbedos were running on multiple processors from multiple manufacturers I would be more inclined to agree with your viewpoint. The code work being done here I think fixes a major flaw in the classic mbed codebase that got pulled into mbedos. Discouraging usage of the workaround code from mbed classic is quite valid in my opinion.

@autopulated
Copy link
Contributor

@BlackstoneEngineering mbed OS is a moving target.

It needs to move in order to be the best it can be, the earlier that movement happens the less painful it'll be.

If you are building a product with the current version of mbed OS I would strongly advise committing all of the dependencies (i.e. the yotta_modules and yotta_targets folders) to your source control, which gives you an easy way to roll back if things break after a yotta update.

For example programs, if not prepared to keep them up to date with the bleeding edge (which would seem to be a useful goal in itself, as it provides additional test coverage for the latest version), please distribute a shrink-wrap file with the example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Forcing serial to be enabled in retarget.cpp prevents the NRF51 from entering low power mode.
7 participants