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

Prevent detect_targets.py tool script from crashing on known targets #6000

Merged
merged 1 commit into from Feb 15, 2018

Conversation

Projects
None yet
5 participants
@daid
Contributor

daid commented Feb 2, 2018

As mut['mcu'] can be "None" on unknown targets, the detect_targets script crashes when one of these boards is connected.

This happens when "mbed-cli detect -vv" is ran when a STEVAL-3DP001V1 board is connected. Which does not provide a html file with a target_id, and thus cannot be looked up in the mbedls platform database.
http://www.st.com/en/evaluation-tools/steval-3dp001v1.html

Notes:

  • Pull requests will not be accepted until the submitter has agreed to the contributer agreement.
  • This is just a template, so feel free to use/remove the unnecessary things

Description

A few sentences describing the overall goals of the pull request's commits.

Status

READY/IN DEVELOPMENT/HOLD

Migrations

If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.

YES | NO

Related PRs

List related PRs against other branches:

branch PR
other_pr_production link
other_pr_master link

Todos

  • Tests
  • Documentation

Deploy notes

Notes regarding the deployment of this PR. These should note any required changes in the build environment, tools, compilers and so on.

Steps to test or reproduce

Outline the steps to test or reproduce the PR here.

Prevent detect_targets.py tool script from crashing on known targets
As mut['mcu'] can be "None" on unknown targets, the detect_targets script crashes when one of these boards is connected.

This happens when "mbed-cli detect -vv" is ran when a STEVAL-3DP001V1 board is connected. Which does not provide a html file with a target_id, and thus cannot be looked up in the mbedls platform database.
http://www.st.com/en/evaluation-tools/steval-3dp001v1.html
@mbed-ci

This comment has been minimized.

mbed-ci commented Feb 2, 2018

User not whitelisted, CI not run.

@cmonr cmonr added the needs: review label Feb 3, 2018

@cmonr cmonr requested review from theotherjimmy and c1728p9 Feb 3, 2018

@cmonr cmonr added needs: work and removed needs: review labels Feb 3, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Feb 3, 2018

@daid I'm not sure I like that solution, since my impression is that all mbed-enabled board should have some sort of html or info on the MSD interface used to identify it.

@c1728p9 I'm wondering if this is because the interface is STLINKV2. Is it possible that ST didn't include the files for this particular board in the firmware?

As a side note, looks like a fun board!

@daid

This comment has been minimized.

Contributor

daid commented Feb 3, 2018

Indeed, there is only a txt file, not the htm file on this board, which includes an STLINKV2 on the board itself.

mbedls is also giving this warning because of it:
WARNING:mbedls.lstools_base:Could not read htm on from usb id 0670FF545250777267104334. Falling back to usb id

Else I would have contributed the proper id to the board database, but I don't think usb ids are proper stable identifiers for boards (I haven't look if this id changes between the 3 STEVAL-3DP001V1 boards we have)

I'm also not entirely sure if this is the correct fix, but this at least prevented the code from crashing, and list the board as incompatible with all default targets.
A different fix would be to detect this case earlier in the code, drop the board from the list and emit a warning that a mock needs to be setup with mbedls

@cmonr cmonr added needs: review and removed needs: work labels Feb 5, 2018

@daid

This comment has been minimized.

Contributor

daid commented Feb 13, 2018

Just dropping by to say that our other STEVAL-3DP001V1 boards indeed have other identifiers, and thus adding it to the mbedls list is not an option.

The question here remains, should mbedls prevent giving None as ['mcu'] or should mbed-os be able to handle this None (as I did in this pull request)

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Feb 13, 2018

@daid I think this is an acceptable solution. The Mbed LS API does not have an "mcu" key, so I think this is the correct factoring.

@cmonr cmonr added needs: CI and removed needs: review labels Feb 14, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Feb 14, 2018

/morph build

@cmonr

This comment has been minimized.

Contributor

cmonr commented Feb 15, 2018

Odd.

/morph build

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Feb 15, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Feb 15, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr merged commit 28ac288 into ARMmbed:master Feb 15, 2018

19 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
mbed-ci-generic Build finished.
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Local littlefs testing has passed
Details
travis-ci/mbed2-ATMEL Local mbed2-ATMEL testing has passed
Details
travis-ci/mbed2-MAXIM Local mbed2-MAXIM testing has passed
Details
travis-ci/mbed2-NORDIC Local mbed2-NORDIC testing has passed
Details
travis-ci/mbed2-NUVOTON Local mbed2-NUVOTON testing has passed
Details
travis-ci/mbed2-NXP Local mbed2-NXP testing has passed
Details
travis-ci/mbed2-RENESAS Local mbed2-RENESAS testing has passed
Details
travis-ci/mbed2-SILICON_LABS Local mbed2-SILICON_LABS testing has passed
Details
travis-ci/mbed2-STM Local mbed2-STM testing has passed
Details
travis-ci/tools Local tools testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment