Skip to content
This repository was archived by the owner on Jun 8, 2021. It is now read-only.

Conversation

@LDong-Arm
Copy link
Collaborator

@LDong-Arm LDong-Arm commented Jun 18, 2020

This PR is created to

  • Add COMPONENT_wifi-ism43362.lib which is for a commonly used WiFi module. This also mean we can get rid of the "quirk" in the Mbed OS CI script to add this driver.
  • Remove support for IDW0* shields which are deprecated and whose driver has not been updated to work with the current Mbed OS.
  • Update README - remove the section "Adding connectivity driver" as there's now no need to add drivers manually.

@LDong-Arm
Copy link
Collaborator Author

@jamesbeyond @evedon This adds wifi-ism43362.lib to resolve the CI limitation, as we discussed.

Now there's no need for users to manually add any WiFi drivers, so there's a bit of clean up in README. @MarceloSalazar

@MarceloSalazar
Copy link
Contributor

Perhaps you should put all the drivers .lib file under a single folder (e.g. "drivers") to keep the root directory clean/small?

@LDong-Arm LDong-Arm force-pushed the module_support_update branch from 2763e43 to 2e72503 Compare June 18, 2020 14:37
@LDong-Arm
Copy link
Collaborator Author

Perhaps you should put all the drivers .lib file under a single folder (e.g. "drivers") to keep the root directory clean/small?

Done

Copy link
Contributor

@evedon evedon left a comment

Choose a reason for hiding this comment

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

LGTM

@jamesbeyond
Copy link
Contributor

once this PR merged, will need https://github.com/ARMmbed/mbed-os-ci/pull/1268 merge as well
Otherwise main CI will fail

UBLOX_EVK_ODIN_W2 is not supported in the latest Mbed OS.
Also there's no need to fetch wifi-ism43362 separately
as it is handled by a .lib file.
@0xc0170
Copy link
Collaborator

0xc0170 commented Jun 22, 2020

Can this be merged (I see couple of dependencies here but do not fully understand the full picture in which order they should get in to have PR ARMmbed/mbed-os#13070 passing) ?

@LDong-Arm
Copy link
Collaborator Author

LDong-Arm commented Jun 23, 2020

Hi @AnttiKauppila @kivaisan @kimlep01,

I'm updating the Jenkinsfile in this PR, but the CI isn't picking it up:

Connecting to https://api.github.com to check permissions of obtain list of LDong-Arm for ARMmbed/mbed-os-example-wifi
Loading trusted files from base branch master at fc56be30e9f0a4c2eff54796c38dce9263203462 rather than 69ffba562a937b404ce65c2d892b6802ce224629
Obtained Jenkinsfile from fc56be30e9f0a4c2eff54796c38dce9263203462

It still uses the Jenkinsfile from the master branch. Is it due to some permission settings in CI?

Reviews on this PR are welcome too, thanks in advance!

@kimlep01
Copy link
Contributor

Hi @AnttiKauppila @kivaisan @kimlep01,

I'm updating the Jenkinsfile in this PR, but the CI isn't picking it up:

Connecting to https://api.github.com to check permissions of obtain list of LDong-Arm for ARMmbed/mbed-os-example-wifi
Loading trusted files from base branch master at fc56be30e9f0a4c2eff54796c38dce9263203462 rather than 69ffba562a937b404ce65c2d892b6802ce224629
Obtained Jenkinsfile from fc56be30e9f0a4c2eff54796c38dce9263203462

It still uses the Jenkinsfile from the master branch. Is it due to some permission settings in CI?

Reviews on this PR are welcome too, thanks in advance!

Hi, Jenkins is set to trust forks from admins or write permissions.

@LDong-Arm
Copy link
Collaborator Author

Hi, Jenkins is set to trust forks from admins or write permissions.

Would you please either commit the Jenkinsfile change for me, or is it possible to give me write permission to this repo? I'm from the PAN team which may probably need permissions in this repo soon anyway. Thanks.

@kimlep01
Copy link
Contributor

Hi, Jenkins is set to trust forks from admins or write permissions.

Would you please either commit the Jenkinsfile change for me, or is it possible to give me write permission to this repo? I'm from the PAN team which may probably need permissions in this repo soon anyway. Thanks.

You should have write access now.

@LDong-Arm
Copy link
Collaborator Author

You should have write access now.

Thanks a lot, it works now 👍

@evedon
Copy link
Contributor

evedon commented Jun 23, 2020

@AnttiKauppila can you merge please?

@LDong-Arm
Copy link
Collaborator Author

@AnttiKauppila can you merge please?

@evedon I'm afraid we need to keep this on hold, because ARMmbed/mbed-os-ci#1268 seems not ready yet. They need to get in at the same time otherwise the Mbed OS CI will fail.
@jamesbeyond I hope that PR can be ready soon?

@LDong-Arm
Copy link
Collaborator Author

LDong-Arm commented Jul 1, 2020

We need to take some risk - merge this first, so we can test ARMmbed/mbed-os-ci#1268. Mbed OS CI will temporarily not work in between them - let's be quick. @jamesbeyond @evedon

@LDong-Arm LDong-Arm merged commit 77052f9 into ARMmbed:master Jul 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants