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

Add device_name for TB_SENSE_12. #5470

Merged
merged 2 commits into from Dec 20, 2017

Conversation

Projects
None yet
8 participants
@ashok-rao
Contributor

ashok-rao commented Nov 9, 2017

Description

device_name key is required to add Cloud Client support / enable bootloader support for this target.

Status

READY

Deploy notes

None.

Adding device_name for TB_SENSE_12.
device_name key is required to add Cloud Client support / enable bootloader support for this target.
@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Nov 9, 2017

@stevew817 @0xc0170 ..Could you please check? Thank you.

cc @screamerbg

@0xc0170 0xc0170 added the needs: CI label Nov 10, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 10, 2017

/morph build

@stevew817

This comment has been minimized.

Contributor

stevew817 commented Nov 10, 2017

LGTM

@mbed-ci

This comment has been minimized.

mbed-ci commented Nov 10, 2017

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@stevew817

This comment has been minimized.

Contributor

stevew817 commented Nov 10, 2017

@0xc0170 The Keil failure above looks weird... Apparently it can't deal with .S files?

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Nov 10, 2017

@theotherjimmy ..does this exporter failure above have something to do with .s and .S as seen in the past? the PR is only adding "device_name" to targets.json..

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 10, 2017

/morph export-build

@amq

This comment has been minimized.

Contributor

amq commented Nov 10, 2017

There is one more target which needs device_name, both of them are covered in #5253

@mbed-ci

This comment has been minimized.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 10, 2017

@ashok-rao Those extensions are only distinguished on POSIX, and uvision only runs on Windows. So, I hope not.

I have seen this before, but I don't recall what the solution was. I'll have to take a look in more detail on Monday.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Nov 10, 2017

@theotherjimmy .. I see.. but I remember I had this issue on Windows some time back and I think others also faced this on their Windows machine.. Issue : #5145 .. Please let us know what you find on Monday! Thanks!

@theotherjimmy theotherjimmy changed the title from Adding device_name for TB_SENSE_12. to Add device_name for TB_SENSE_12. Nov 13, 2017

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Nov 13, 2017

@theotherjimmy @0xc0170 .. why is only uVision failing for this? I can see others are passing from Jenkins log...??

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 13, 2017

@ashok-rao This change only affects the Uvision Exporter. I would be surprised if others failed because of this.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 14, 2017

@ashok-rao Can you reproduce this issue locally please? Use export -t TB_SENSE_12 -i uvision

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Nov 14, 2017

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Nov 14, 2017

Hi @0xc0170 . It seems to build fine on my local machine.. Steps I followed:
I cloned the latest master of Mbed OS:

C:\Ashok\blinky\mbed-os-example-blinky\mbed-os>git log
commit 1394bf95f19fdb0cbf569359bbac0b02efec5d87

Made the change..
tb_sense_12_add_device_name

then did mbed export :

C:\Ashok\blinky\mbed-os-example-blinky>mbed export -i uvision -m TB_SENSE_12 -vv
[mbed] Working path "C:\Ashok\blinky\mbed-os-example-blinky" (program)
[mbed] Exec "c:\python27\python.exe -u C:\Ashok\blinky\mbed-os-example-blinky\mbed-os\tools\project.py -i uvision -m TB_SENSE_12 --source ." in C:\Ashok\blinky\mbed-os-example-blinky
Scan: .

C:\Ashok\blinky\mbed-os-example-blinky>

And built it with MDK. I'm attaching build log below:
TB_SENSE_12_MDK_build_log.txt

Just checking if CI uses this version of ARMCC: Mine is this version:
*** Using Compiler 'V5.06 update 5 (build 528)', folder: 'C:\Keil_v5\ARM\ARMCC\Bin'

Edit:
Just checking the CI logs & it is using : *** Using Compiler 'V5.06 update 3 (build 300)', folder: 'C:\mbed_tools\Keil_v5\ARM\ARMCC\Bin' .
Does the compiler build version have anything to do with the .s and the .S failures?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Nov 14, 2017

I would not suspect this is related to .s or .S files. __UVISION_VERSION should not be even present as uvisor is not supported for ARMCC.

I manually fetched this PR, and was able to build for uvision using exported project. After I installed the a pack for this device . 😕

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 14, 2017

Does the compiler build version have anything to do with the .s and the .S failures?

I don't recall, so it might.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 14, 2017

@ashok-rao In general, it's more helpful to post code snippets in ``` blocks compared to screenshots. The advantage is that we can copy-paste from those blocks.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 14, 2017

@0xc0170 I don't know what you're on about. The failure is in uVision, which, as far as I know, always emits that macro. the example that is failing is blinky. Where are you getting uvisor? it's just not present anywhere in the logs.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 14, 2017

After I installed the a pack for this device

What error did you get before installing the pack for the device?


Everyone:
Do not claim you cannot reproduce the problem, after trying to build using the GUI. It may not do exactly the same thing as the batch build. Please use the batch build before you claim that you cannot reproduce the problem.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Nov 14, 2017

@0xc0170 , same with me, I had to install the DFP for the EFR32MG12P332F1024xx125. Without the pack, MDK was selecting a generic M4_FP target & still builds succesfully. Not sure what you refer to regarding uvisor ?

@theotherjimmy , thanks for the note on screenshots, I just wanted to highlight the line numbers as well since targets.json has changed from my local copy to the latest master..so, just wanted to specify the line numbers where I made the change, hence the screenshot!
Attaching the batch file below that I generated from MDK (Under target options -> build -> generate batch file). It builds fine for me in the local system though & generates an AXF after successful build (I didn't convert that to bin)..
mbed-os-example-blinky.zip

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Nov 14, 2017

@ashok-rao Then you could use a git diff, it would include line numbers. For example:

diff --git a/targets/targets.json b/targets/targets.json
index 0a8549f00..44eb9050c 100755
--- a/targets/targets.json
+++ b/targets/targets.json
@@ -2912,6 +2912,7 @@
     },
     "TB_SENSE_12": {
         "inherits": ["EFR32MG12P332F1024GL125"],
+        "device_name": "EFR32MG12P332F1024GL125",
         "device_has": ["ANALOGIN", "I2C", "I2CSLAVE", "I2C_ASYNCH", "INTERRUPTIN", "LOWPOWERTIMER", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_ASYNCH", "SLEEP", "SPI", "SPISLAVE", "SPI_ASYNCH", "STDIO_MESSAGES", "TRNG", "FLASH"],
         "forced_reset_timeout": 5,
         "config": {

mine is also different, at line 2912 (between the @@). Even better, with a diff I can copy it to git am and git will apply it to my working copy. That said, I don't think line numbers matter much here...


Under target options -> build -> generate batch file

I don't think that's how CI runs the builds. The CI builds issuing the command in the build method. That may produce a different result than the method you used.

That being said, I would be surprised if it did.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Nov 14, 2017

Thanks for the git diff notes! Will try to use where applicable.

Regarding batch files, I don't suspect either generation method would produce major differences though in the batch file generated. That being said, I don't have reach into the CI builds, so I cannot check that end. Someone with access could help us here!

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Dec 1, 2017

Nice. Thank you @cmonr . Awaiting your inputs.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Dec 1, 2017

@ashok-rao It looks like the issue is on our side, and will have to wait until Monday for a fix to be tested.

No action on your end is needed at the moment.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Dec 11, 2017

@cmonr .. any news on this, please?

@cmonr

This comment has been minimized.

Contributor

cmonr commented Dec 11, 2017

@ashok-rao Looks like the modification made it in this weekend. I'm going to rerun the job to verify.

/morph export-build

@mbed-ci

This comment has been minimized.

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 11, 2017

/morph export-build

@mbed-ci

This comment has been minimized.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Dec 12, 2017

?? Same error again..!! @cmonr ..

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Dec 15, 2017

@cmonr @kegilbert ..bump!

@mbed-ci

This comment has been minimized.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Dec 15, 2017

Hmm, I don't see what failed here.. Looks like all tests have passed from the CI logs.. @0xc0170 @cmonr @kegilbert can you see what has failed?

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 15, 2017

We've been seeing some Jenkins related failures intermittently, sorry for the delay.
/morph export-build

@cmonr

This comment has been minimized.

Contributor

cmonr commented Dec 15, 2017

@ashok-rao I ran a limited manual build, and things finally look good on our side.

Barring any further strange Jenkins issues, this last build should do it!

@mbed-ci

This comment has been minimized.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Dec 15, 2017

@cmonr @kegilbert .. crashed again!! And this time it's a different platform altogether.

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 15, 2017

This looks like an actual compilation error, running it by @geky since it looks like a filesystem error. That it only failed on that one platform makes me think it's another one-off error though...

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 15, 2017

My local compiler version is slightly off from the CI version but builds the export correctly. Trying again to see if it replicates and allocate the build node to debug further if it happens again.
/morph export-build

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 15, 2017

Offline static node got assigned, this time for real.
/morph export-build

@mbed-ci

This comment has been minimized.

mbed-ci commented Dec 15, 2017

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 16, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Dec 17, 2017

Build : SUCCESS

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

Triggering tests

/morph test

@mbed-ci

This comment has been minimized.

@ashok-rao

This comment has been minimized.

Contributor

ashok-rao commented Dec 17, 2017

Awesome! Finally..! @0xc0170 @adbridge ..over to you to take this PR further. Thank you @kegilbert @cmonr ..

@0xc0170 0xc0170 added ready for merge and removed needs: work labels Dec 18, 2017

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Dec 18, 2017

No problem, sorry for the pain and delay @ashok-rao .

@0xc0170 0xc0170 merged commit 9ebefcd into ARMmbed:master Dec 20, 2017

6 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment