Update Cmsis-pack-manager to 0.2.3 #8757

theotherjimmy · 2018-11-15T20:35:45Z

Description

This comes with quite a few features:

Variant parsing
Memory permission bits for:
- Read
- Write
- Execute
- Secure
- Non-Secure
- Non-Secure Callable
- Peripheral
Memory default information
Descriptions of core as either:
- Semetric Multi-core
- Asymetric Multi-core

Further, The index has been re-formatted to be human-readable and all
keys are sorted so that diffs are also human-readable. Be aware, it's
big.

~~NOTE: This will conflict with #8607~~ Rebased after #8607

Pull request type

[ ] Fix
[x] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

cmonr · 2018-11-15T23:14:39Z

Looks at changes
Sees 450k+ changes
Uhhh....

Sees tools/arm_pack_manager/index.json was updated
Oh. Carry on.

@theotherjimmy Would/could that file benefit from getting new lines added to it, similar to how target.json was updated, so that the diff generated isn't 99.9% if the PR when it's updated?

bridadan

Few clarifications

tools/config/__init__.py

bridadan · 2018-11-15T23:30:59Z

tools/config/__init__.py

+        ]
+        if all_matching_memories:
+            memory = sorted(
+                all_matching_memories, key=self._memory_ordering


I'm a bit tripped up on the key function here. Is this essentially doing the following for the comparison:

"
For each memory in all_matching_memories, the one that is the "least" is the one that:

Has the smallest value for memory['default']

If they are the same, pick the smallest value for memory['size']

If that is the same, pick the smallest value for memory['start']

"

@bridadan I was under the impression that it would be largest first. If I need a quick reverse to make them all "largest" then that's what I'll do.

sorted will return an ascending order sorted list by default: https://docs.python.org/3/howto/sorting.html. So that would mean the first one in the list would be the "smallest" if I'm interpreting the doc correctly.

Right. Adding a reverse now.

cmonr

Nice!

cmonr · 2018-11-15T23:20:27Z

requirements.txt

@@ -19,3 +19,4 @@ six==1.11.0
 git+https://github.com/armmbed/[email protected]
 mbed-cloud-sdk==2.0.1
 icetea>=1.0.2,<1.1
+cmsis-pack-manager>=0.2.2,<0.3.0


cmonr · 2018-11-15T23:30:53Z

tools/arm_pack_manager/__init__.py


    def _get_sectors(self, device):
        """Extract sector sizes from device FLM algorithm

-        Will return None if there is no algorithm, pdsc URL formatted in correctly
+        Will return None if there is no algorithm, pdsc URL formatted in


Not sure why, but this feels like weird english.

Is it missing an or?

cmonr · 2018-11-15T23:32:55Z

tools/arm_pack_manager/pack_manager.py

@@ -1,192 +0,0 @@
-from __future__ import print_function, division, absolute_import


Goodnight sweet prince

cmonr · 2018-11-15T23:36:32Z

@ARMmbed/mbed-os-test-team Heads up. A new python module is being added into mbed-os

https://github.com/ARMmbed/mbed-os/pull/8757/files#diff-b4ef698db8ca845e5845c4618278f29aR22

@theotherjimmy Do you know if cmsis-pack-manager has a minimum Py2/Py3 versions?

theotherjimmy · 2018-11-16T15:50:45Z

@cmonr It's compatible with most Python versions >= 2.7

theotherjimmy · 2018-11-16T15:52:04Z

@cmonr

Would/could that file benefit from getting new lines added to it, similar to how target.json was updated, so that the diff generated isn't 99.9% if the PR when it's updated?

from the top comment:

Further, The index has been re-formatted to be human-readable and all
keys are sorted so that diffs are also human-readable. Be aware, it's
big.

cmonr · 2018-11-16T16:12:00Z

Further, The index has been re-formatted to be human-readable and all
keys are sorted so that diffs are also human-readable. Be aware, it's
big.

Ha. Silly GitHub not wanting to load the file...

NirSonnenschein · 2018-11-18T13:47:06Z

/morph build

NirSonnenschein · 2018-11-18T13:49:45Z

@0xc0170 , @cmonr I've advanced this one, any reason why it still needs work?

mbed-ci · 2018-11-18T13:57:25Z

Build : FAILURE

Build number : 3666
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8757/

NirSonnenschein · 2018-11-18T20:20:50Z

@theotherjimmy this PR ran into a strange build failure, mind taking a look:
Traceback (most recent call last):
File "/usr/local/lib/python2.7/dist-packages/mbed/mbed.py", line 3338, in main
status = pargs.command(pargs)
File "/usr/local/lib/python2.7/dist-packages/mbed/mbed.py", line 1985, in thunk
return command(**argv)
File "/usr/local/lib/python2.7/dist-packages/mbed/mbed.py", line 2769, in test_
icetea_supported = program.requirements_contains('icetea')
File "/usr/local/lib/python2.7/dist-packages/mbed/mbed.py", line 1603, in requirements_contains
with open(os.path.join(req_path, req_file), 'r') as f:
IOError: [Errno 2] No such file or directory: '/builds/ws/mbed-os-build-matrix/3666/default_GCC_ARM/requirements.txt'

0xc0170 · 2018-11-19T10:40:10Z

NOTE: The history is not clean yet. I plan to squash/rewrite commit
messages to meet commit message standards.

@NirSonnenschein labeled as needs: work because of this

OPpuolitaival · 2018-11-19T13:54:21Z

tools/arm_pack_manager/__init__.py

-                    flm = pack.open(filename)
+                    flm = pack.open(
+                        algo["file_name"]
+                        .replace("\\\\", "/")


This works also in Windows?

Maybe. I'm not sure.

OPpuolitaival

Is this tested also in Windows machine?

theotherjimmy · 2018-11-19T15:40:06Z

@OPpuolitaival This has not been tested on a windows machine that's next on my list.

theotherjimmy · 2018-11-19T15:41:27Z

@0xc0170 The history is actually not so bad. I already did some squashing/rewriting.

deepikabhavnani · 2018-11-20T12:43:04Z

@theotherjimmy - as discussed internally should we move this to 5.12 as it will have broader impact on devices?

theotherjimmy · 2018-11-20T15:02:21Z

@deepikabhavnani Perhaps "broader impact" is not the correct phrase. "it's a big risk" as it can impact many systems is probably what you were thinking. Especially because it's supposed to have no impact on any devices.

deepikabhavnani · 2018-11-20T15:16:23Z

@theotherjimmy - Yes agree risk is higher and we might uncover some unknowns as well..

mbed-ci · 2019-03-04T20:46:19Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARMC6

theotherjimmy · 2019-03-04T22:36:15Z

So, the NUMAKER_PFM_M2351 now has 2x ROMs, each 1/2 the size that it used to have, that are not mapped contiguously. That's very strange indeed.

cmonr · 2019-03-04T23:22:33Z

CI started

mbed-ci · 2019-03-05T00:29:00Z

Test run: FAILED

Summary: 1 of 9 test jobs failed
Build number : 12
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARMC6

cmonr · 2019-03-05T00:32:19Z

Looking into this, but I don't think the jenkins-ci/mbed-os-ci_build-ARMC6 failure is due to the PR.

mbed-ci · 2019-03-05T03:10:05Z

Test run: FAILED

Summary: 1 of 9 test jobs failed
Build number : 13
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARMC6

0xc0170 · 2019-03-05T07:04:43Z

Looking into this, but I don't think the jenkins-ci/mbed-os-ci_build-ARMC6 failure is due to the PR.

Any findings? I'll reviewed changes, do not look like this would hit assert in the linker script. I'll fetch this PR and reproduce locally

0xc0170 · 2019-03-05T09:08:39Z

So, the NUMAKER_PFM_M2351 now has 2x ROMs, each 1/2 the size that it used to have, that are not mapped contiguously. That's very strange indeed.

This is what I have noticed (correction - RAM regions are split now) and possibly that is why this assert with ARMC6 now. @ARMmbed/team-nuvoton Should we just changed back to only one RAM region (as it was) and this will be addressed separately?

Here in this PR:

            "IRAM1": {
                "access": {
                    "execute": false, 
                    "non_secure": false, 
                    "non_secure_callable": false, 
                    "peripheral": false, 
                    "read": true, 
                    "secure": false, 
                    "write": true
                }, 
                "default": true, 
                "size": 32768, 
                "start": 536870912, 
                "startup": false
            }, 
            "IRAM2": {
                "access": {
                    "execute": false, 
                    "non_secure": false, 
                    "non_secure_callable": false, 
                    "peripheral": false, 
                    "read": true, 
                    "secure": false, 
                    "write": true
                }, 
                "default": false, 
                "size": 65536, 
                "start": 805339136, 
                "startup": false
            },

On master:

"memory": {
    "IRAM1": {"start": "0x20000000", "size": "0x18000"},

This asserts ScatterAssert(ImageLimit(ARM_LIB_HEAP) <= (MBED_RAM_APP_START + MBED_RAM_APP_SIZE))

Heap goes in this PR to RAM1 region that is 3x smaller and does not fit.

If we change regions in cmsis pack index, we need to change also linker scripts. I've checked all 3 linkers scripts, all of them contain one RAM region. I would revert RAM region change in this PR, and will request a new issue to address this (or fix cmsis pack).

0xc0170 · 2019-03-05T09:43:26Z

Locally reverted RAM2 change for this nuvoton target, builds without error. I'll push this commit here, restart testing when we can. We will need to create an issue for this (RAM2 region addition).

Build successes:
  * NUMAKER_PFM_M2351::ARMC6::MBED-BUILD
  * NUMAKER_PFM_M2351::ARMC6::TESTS-MBED_PLATFORM-SYSTEM_RESET

Revert latest change to index. Linker scripts follow one RAM region. If index is updated, requires further changes in the target that should be done separately.

0xc0170 · 2019-03-05T09:53:40Z

I pushed a fix (using one RAM region, should have same attributes as it was). Builds locally, Same RAM1 as it was.

0xc0170 · 2019-03-05T09:55:59Z

CI started

mbed-ci · 2019-03-05T12:26:02Z

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 14
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

0xc0170 · 2019-03-05T12:28:20Z

@theotherjimmy Can you review the failures? It's not related to this change-set but rather on master already. The issue is being tracked here #9938 .

cmonr · 2019-03-05T21:48:50Z

CI restarted

mbed-ci · 2019-03-06T00:42:15Z

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 15
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

cmonr · 2019-03-06T05:18:29Z

CI restarted.
Merge commit needs to use updated master

mbed-ci · 2019-03-06T08:35:10Z

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 16
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

0xc0170 · 2019-03-06T12:23:13Z

Exporters restarted (license issue)

alekla01 · 2019-03-06T13:17:03Z

Restarted exporters

cmonr added needs: work needs: preceding PR release-version: 5.11.0-rc1 labels Nov 15, 2018

cmonr requested review from bridadan and cmonr November 15, 2018 23:11

bridadan reviewed Nov 15, 2018

View reviewed changes

cmonr approved these changes Nov 15, 2018

View reviewed changes

bridadan approved these changes Nov 16, 2018

View reviewed changes

0xc0170 approved these changes Nov 16, 2018

View reviewed changes

NirSonnenschein added the needs: CI label Nov 18, 2018

0xc0170 removed the needs: CI label Nov 19, 2018

OPpuolitaival reviewed Nov 19, 2018

View reviewed changes

0xc0170 added release-version: 5.12.0-rc1 and removed release-version: 5.11.0-rc1 labels Nov 20, 2018

Manually re-combine NUMAKER_PFM_M2351 memories

288fc2f

nuvoton m2351: fix RAM regions - only one RAM1

812812a

Revert latest change to index. Linker scripts follow one RAM region. If index is updated, requires further changes in the target that should be done separately.

0xc0170 added needs: CI and removed needs: work labels Mar 5, 2019

theotherjimmy mentioned this pull request Mar 5, 2019

Fail to update cmsis pack index.json #9375

Closed

Remove prints and lint warnings

a045c45

0xc0170 added ready for merge and removed needs: CI labels Mar 6, 2019

0xc0170 merged commit c37ac83 into ARMmbed:master Mar 6, 2019

0xc0170 removed the ready for merge label Mar 6, 2019

ccli8 mentioned this pull request Mar 8, 2019

M2351: Support memory custom partition #10004

Merged

		@@ -1,192 +0,0 @@
		from __future__ import print_function, division, absolute_import

Update Cmsis-pack-manager to 0.2.3 #8757

Update Cmsis-pack-manager to 0.2.3 #8757

Uh oh!

Conversation

theotherjimmy commented Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

cmonr commented Nov 15, 2018

Uh oh!

bridadan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bridadan Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmonr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmonr commented Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy commented Nov 16, 2018

Uh oh!

theotherjimmy commented Nov 16, 2018

Uh oh!

cmonr commented Nov 16, 2018

Uh oh!

NirSonnenschein commented Nov 18, 2018

Uh oh!

NirSonnenschein commented Nov 18, 2018

Uh oh!

mbed-ci commented Nov 18, 2018

Uh oh!

NirSonnenschein commented Nov 18, 2018

Uh oh!

0xc0170 commented Nov 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OPpuolitaival left a comment

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Nov 19, 2018

Uh oh!

theotherjimmy commented Nov 19, 2018

Uh oh!

deepikabhavnani commented Nov 20, 2018

Uh oh!

theotherjimmy commented Nov 20, 2018

Uh oh!

deepikabhavnani commented Nov 20, 2018

Uh oh!

mbed-ci commented Mar 4, 2019

Uh oh!

theotherjimmy commented Mar 4, 2019

Uh oh!

cmonr commented Mar 4, 2019

Uh oh!

mbed-ci commented Mar 5, 2019

Uh oh!

theotherjimmy commented Nov 15, 2018 •

edited

Loading

bridadan Nov 15, 2018 •

edited

Loading

cmonr commented Nov 15, 2018 •

edited

Loading

0xc0170 commented Mar 5, 2019 •

edited

Loading