Skip to content

Remove unsupported targets #12810

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 30 commits into from
Apr 22, 2020
Merged

Remove unsupported targets #12810

merged 30 commits into from
Apr 22, 2020

Conversation

MarceloSalazar
Copy link

@MarceloSalazar MarceloSalazar commented Apr 15, 2020 •
edited
Loading

Summary of changes

This is the second of a series of PRs (first #12775) to remove targets that are not supported or retired as explained in recent blog post.

There are individual commits to remove targets from targets.json and files/folders for the corresponding targets.

Impact of changes

The following targets are being removed:

  • MTBs (most)
  • nRF51/52
    • Ublox NINA B1
    • LAIRD_BL600, BL652, BL654
    • OSHCHIP
    • ACONNO_ACN52832
    • MURATA_WSM_BL241
  • LPC11u (most)
  • Misc unsupported

Migration actions required

  • The targets listed above continue to be available in the 5.15 branch.

Documentation

  • None.
    We'll update os.mbed.com/platforms with the list of supported platforms for Mbed 6

Pull request type 

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[X] 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

Expect regressions tests to be done by CI as part of this PR.

Reviewers

@ARMmbed/mbed-os-maintainers

@mergify mergify bot added the needs: work label Apr 15, 2020
@mergify
Copy link

mergify bot commented Apr 15, 2020

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

@ciarmcom ciarmcom requested review from maclobdell and a team April 15, 2020 17:00
@ciarmcom
Copy link
Member

@MarceloSalazar, thank you for your changes.
@maclobdell @ARMmbed/mbed-os-wan @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-test @ARMmbed/mbed-os-tools please review.

@pilotak
Copy link
Contributor

pilotak commented Apr 17, 2020

what a shame that the BLUEPILL (STM32F103C8) is not going to be supported, there are a lot of them in people's drawers. OS5 worked fine on it.

@MarceloSalazar
Copy link
Author

@pilotak no worries, the BLUEPILL continues to be supported in the 5.15 branch and in the future, you can use it with Mbed OS 6 using custom targets

@jeromecoutant
Copy link
Collaborator

But why remove all files...?
Removing only target from targets.json was sufficient ?

@MarceloSalazar
Copy link
Author

@jeromecoutant we agreed to remove all unused folders/files to keep the repo clean and whether possible ensure there isn't unused code left behind.

@pilotak
Copy link
Contributor

pilotak commented Apr 17, 2020

@MarceloSalazar i can understand but it was good to have for begginers, i doubt that someone starting with the mbed would want to create a custom target, they just need it to work out of the box. Nevermind, hopefully they will find the porting guide.

@MarceloSalazar
Copy link
Author

Rebased and fixed conflicts.
@ARMmbed/mbed-os-maintainers please review

@0xc0170 0xc0170 removed request for a team April 18, 2020 06:49
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 20, 2020

Started CI meanwhile

@0xc0170 0xc0170 requested a review from bulislaw April 20, 2020 10:12
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 20, 2020

I was just reviewing nightly and one of these targets failed (MTB_LAIRD_BL600) so won't create a new issue as it is going to be removed soon here.

@mbed-ci
Copy link

mbed-ci commented Apr 20, 2020

Test run: FAILED

Summary: 2 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_wisun-mesh-test
  • jenkins-ci/mbed-os-ci_greentea-test

@mbed-ci
Copy link

mbed-ci commented Apr 21, 2020

Test run: SUCCESS

Summary: 7 of 7 test jobs passed
Build number : 2
Build artifacts

@VeijoPesonen VeijoPesonen mentioned this pull request Apr 21, 2020
Merged
ghost
ghost reviewed Apr 21, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

There are no targets removed here.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 22, 2020

There are no targets removed here.

Where? I dont see any specific commit or line referenced in this comment?

@ghost
Copy link

ghost commented Apr 22, 2020 •
edited by ghost
Loading

@0xc0170 : sorry. I thought i did click comment on the commit. Here it is 21e364e It does not do what it says it does.

bulislaw
bulislaw approved these changes Apr 22, 2020
View reviewed changes
Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

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

Ok for the list of targets that's being removed

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 22, 2020

@0xc0170 : sorry. I thought i did click comment on the commit. Here it is 21e364e It does not do what it says it does.

Good catch.

I'll merge this as it is. The change is minimalist. I'll leave a note in the merge commit.
This type of clean-up are hard to time with other changes ongoing, causing conflicts with others.

@0xc0170 0xc0170 merged commit b622a25 into ARMmbed:master Apr 22, 2020
@mergify mergify bot removed the ready for merge label Apr 22, 2020
@ghost
Copy link

ghost commented Apr 22, 2020 •
edited by ghost
Loading

Lots of NRF5* targets were removed in the original version of this PR that are no longer removed. What happened with that? I didn't see any discussion as to why that changed.

I had made a custom target for one of them that it doesn't look like I'll need just yet.

@MarceloSalazar
Copy link
Author

@jrobeson I believe many nrf5* targets have been removed in this PR
See files here: https://github.com/ARMmbed/mbed-os/pull/12810/files
For example:
TARGET_MCU_NRF51822_UNIFIED/TARGET_MTB_LAIRD_BL600/PinNames.h
TARGET_MCU_NRF52832/TARGET_MTB_ACONNO_ACN52832/PinNames.h
TARGET_NRF52/TARGET_MCU_NRF52832/TARGET_MTB_MURATA_WSM_BL241/PinNames.h

Note there is another PR coming to clean-up more targets...

@MarceloSalazar MarceloSalazar deleted the platform_cleanup branch April 22, 2020 10:49
@0xc0170 0xc0170 mentioned this pull request Apr 22, 2020
Merged
@ghost
Copy link

ghost commented Apr 22, 2020

@MarceloSalazar : i was just wondering why it changed in this particular PR. The original version removed NRF51 dongle among many others.

@MarceloSalazar
Copy link
Author

@jrobeson you're right - there was a problem with commits and some of them disappeared...
We'll get this fixed in the upcoming PR :)

@ghost ghost mentioned this pull request Apr 23, 2020
Closed
@MarceloSalazar MarceloSalazar mentioned this pull request Apr 24, 2020
Merged
This was referenced May 6, 2020
Merged
Merged
nabilbendafi added a commit to nabilbendafi/Teensy that referenced this pull request Sep 23, 2021
@nabilbendafi
feat: update Mbed
218d281 
- Use latest Mbed with TEENSY3_1 support (remove by
ARMmbed/mbed-os#12810)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bulislaw bulislaw bulislaw approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

@maclobdell maclobdell Awaiting requested review from maclobdell

Assignees
No one assigned
Labels
BREAKING-CHANGE release-version: 6.0.0-beta-1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@MarceloSalazar @ciarmcom @pilotak @jeromecoutant @0xc0170 @mbed-ci @bulislaw @evandavey @kiranshila @stormon-force