BlockDevices: specify mbed namespace where needed #14031

facchinm · 2020-12-11T13:37:59Z

Summary of changes

Including any of these headers outside mbed namespace (or with MBED_NO_GLOBAL_USING_DIRECTIVE macro) would trigger compilation errors.
Since most of them were already ported to the full mbed:: specifier, apply this to all missing arguments.

Please not that this in not needed by other blockdevices since they are already under mbed namespace.

Impact of changes

Migration actions required

Documentation

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] 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

ciarmcom · 2020-12-11T14:00:24Z

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

evedon · 2020-12-11T14:30:47Z

#13568 is also addressing namespace issues and there is some overlap.
But otherwise, code change looks good

facchinm · 2020-12-11T14:51:34Z

@evedon sorry, missed that one 🙂 Happy to rebase as soon as it gets merged (or merge them together)

mergify · 2021-01-05T11:29:20Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

0xc0170 · 2021-01-05T12:34:39Z

@evedon sorry, missed that one 🙂 Happy to rebase as soon as it gets merged (or merge them together)

As it is not proceeding. I requested confirmation if it goes ahead or we shall make this PR to include both and fix it altogether?

facchinm · 2021-01-05T13:43:37Z

@0xc0170 I'm merging the two patches and force pushing here

0xc0170

One question: TaskBase is in the events namespace. Should also PolledQueue to be or TaskBase should be changed?

Otherwise LGTM

facchinm · 2021-01-05T17:00:19Z

One question: TaskBase is in the events namespace. Should also PolledQueue to be or TaskBase should be changed?

Ah, good question... I just really rebased both the patches and applied the one from #13568 as-is.
Tomorrow I'll check if namespacing TaskBase is also needed.

facchinm · 2021-01-08T09:50:51Z

I can confirm that this patch is enough to include PolledQueue without any other action needed.
Sample project:

mbed_app.json

{
  "macros": [
    "MBED_NO_GLOBAL_USING_DIRECTIVE"
  ],
  "target_overrides": {
....

main.cpp

#include "mbed.h"
#include <PolledQueue.h>
int main() {}

This compiles with this branch while breaks on master
Should this be added to the testsuite @0xc0170 ?

ciarmcom · 2021-01-19T12:51:29Z

This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-core, please complete review of the changes to move the PR forward. Thank you for your contributions.

0xc0170 · 2021-01-19T13:05:40Z

I am now going through stale issues, this one I've missed as well.

Should this be added to the testsuite @0xc0170 ?

Yes, we should test MBED_NO_GLOBAL_USING_DIRECTIVE. Would you be able to add a test case?

0xc0170 · 2021-01-20T15:03:50Z

Started CI to get early results.

mbed-ci · 2021-01-20T16:32:55Z

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	❌
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	❌
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️

ciarmcom · 2021-01-31T16:51:01Z

This pull request has automatically been marked as stale because it has had no recent activity. @facchinm, please carry out any necessary work to get the changes merged. Thank you for your contributions.

facchinm · 2021-02-01T09:34:47Z

I think CI failure is totally unrelated with the commit (it breaks on DISCO_L475VG_IOT01A while compiling NFC_EEPROM example, the error is [Fatal Error] EEPROMDriver.cpp@18,10: m24sr_driver.h: No such file or directory ).
We'll add tests as per @0xc0170 request

LDong-Arm · 2021-02-01T09:58:17Z

I think CI failure is totally unrelated with the commit (it breaks on DISCO_L475VG_IOT01A while compiling NFC_EEPROM example, the error is [Fatal Error] EEPROMDriver.cpp@18,10: m24sr_driver.h: No such file or directory ).
We'll add tests as per @0xc0170 request

It should be fixed by #14092 (merged a moment ago), CI needs a rerun.

pennam · 2021-02-02T08:01:11Z

@0xc0170 we are trying to find out the best way to test this issue. As this is not a runtime error adding a greentea test may not be the best option.
On master branch running mbed compile --library -D MBED_NO_GLOBAL_USING_DIRECTIVE build breaks compiling PolledQueue.cpp; moreover on this branch running mbed test with
MBED_NO_GLOBAL_USING_DIRECTIVE` a lot of test cases are failing to build.

Do you think is worth to add this build check into CI?

Verifying that all tests builds correctly with MBED_NO_GLOBAL_USING_DIRECTIVE active should give us the best coverage.

mbed_test_build_failures.log

added namespaces in various files

pennam · 2021-02-02T11:52:25Z

rebased to rerun CI

0xc0170 · 2021-02-02T11:57:30Z

Ci started

mbed-ci · 2021-02-02T16:21:31Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	✔️

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Dec 11, 2020

ciarmcom requested review from a team December 11, 2020 14:00

ciarmcom added the needs: review label Dec 11, 2020

mergify bot added needs: work and removed needs: review labels Jan 5, 2021

0xc0170 mentioned this pull request Jan 5, 2021

fix namespaces for MBED_NO_GLOBAL_USING_DIRECTIVE feature #13568

Closed

facchinm force-pushed the blockdevices_namespaces branch from 6a7c410 to 4595bff Compare January 5, 2021 13:48

0xc0170 added needs: review and removed needs: work labels Jan 5, 2021

0xc0170 reviewed Jan 5, 2021

View reviewed changes

ciarmcom added the stale Stale Pull Request label Jan 19, 2021

mergify bot added needs: work and removed needs: review labels Jan 20, 2021

ciarmcom removed the stale Stale Pull Request label Jan 20, 2021

ciarmcom added the stale Stale Pull Request label Jan 31, 2021

LDong-Arm mentioned this pull request Feb 1, 2021

DO NOT MERGE: CI only: rebase feature-tf-m-1.2-integration #14216

Closed

facchinm and others added 2 commits February 2, 2021 12:49

BlockDevices: specify mbed namespace where needed

8ca0e9f

fix namespaces for MBED_NO_GLOBAL_USING_DIRECTIVE feature

6b0215c

added namespaces in various files

pennam force-pushed the blockdevices_namespaces branch from 4595bff to 6b0215c Compare February 2, 2021 11:51

ciarmcom removed the stale Stale Pull Request label Feb 2, 2021

0xc0170 approved these changes Feb 4, 2021

View reviewed changes

0xc0170 added ready for merge and removed needs: work labels Feb 4, 2021

0xc0170 merged commit e6565a4 into ARMmbed:master Feb 4, 2021

mergify bot removed the ready for merge label Feb 4, 2021

mbedmain added release-version: 6.8.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Feb 16, 2021

pennam mentioned this pull request Jul 1, 2021

SPIFBlockDevice: Add mbed namespace to BD_ERROR_DEVICE_ERROR #14849

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BlockDevices: specify mbed namespace where needed #14031

BlockDevices: specify mbed namespace where needed #14031

facchinm commented Dec 11, 2020

ciarmcom commented Dec 11, 2020

evedon commented Dec 11, 2020

facchinm commented Dec 11, 2020

mergify bot commented Jan 5, 2021

0xc0170 commented Jan 5, 2021

facchinm commented Jan 5, 2021

0xc0170 left a comment

facchinm commented Jan 5, 2021

facchinm commented Jan 8, 2021

ciarmcom commented Jan 19, 2021

0xc0170 commented Jan 19, 2021

0xc0170 commented Jan 20, 2021

mbed-ci commented Jan 20, 2021

ciarmcom commented Jan 31, 2021

facchinm commented Feb 1, 2021

LDong-Arm commented Feb 1, 2021

pennam commented Feb 2, 2021

pennam commented Feb 2, 2021

0xc0170 commented Feb 2, 2021

mbed-ci commented Feb 2, 2021

BlockDevices: specify mbed namespace where needed #14031

BlockDevices: specify mbed namespace where needed #14031

Conversation

facchinm commented Dec 11, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Dec 11, 2020

evedon commented Dec 11, 2020

facchinm commented Dec 11, 2020

mergify bot commented Jan 5, 2021

0xc0170 commented Jan 5, 2021

facchinm commented Jan 5, 2021

0xc0170 left a comment

Choose a reason for hiding this comment

facchinm commented Jan 5, 2021

facchinm commented Jan 8, 2021

ciarmcom commented Jan 19, 2021

0xc0170 commented Jan 19, 2021

0xc0170 commented Jan 20, 2021

mbed-ci commented Jan 20, 2021

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

ciarmcom commented Jan 31, 2021

facchinm commented Feb 1, 2021

LDong-Arm commented Feb 1, 2021

pennam commented Feb 2, 2021

pennam commented Feb 2, 2021

0xc0170 commented Feb 2, 2021

mbed-ci commented Feb 2, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts