Skip to content
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 Lock app example for infineon P6 board #8268

Merged
merged 11 commits into from
Aug 12, 2021

Conversation

praveenCY
Copy link
Contributor

@praveenCY praveenCY commented Jul 9, 2021

Problem

There is no Support for Infineon P6 board on Matter

Change overview

Adding Support for Infineon CY8CKIT-062S2-43012 board on Matter along with Lock app example

  • Supported Only WiFi and BLE connectivity.
  • There is only one examples in this PR: The lock application.
  • Detailed steps for Building. Flashing along with tools required added in Example README.md

Testing

How was this tested? (at least one bullet point required)

  • If new unit tests are not added, why not?
    Unit tests are not support as of now and will be covered as part of next release update

  • If new integration tests are not added, why not?
    Integration and CI build support will be added as part of next update

  • If manually tested, what platforms controller and device platforms were manually tested, and how?
    CY8CKIT-062S2-43012 + Raspberry Pi4 (chip-device-ctrl)

Detailed steps are added in lock app README.md

Build command
./scripts/examples/gn_p6_example.sh examples/lock-app/p6

P6_Lock_app_logs.txt

@CLAassistant
Copy link

CLAassistant commented Jul 9, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

I didn't review the src/platform things very carefully....

The main question here is what's going on with the third_party repo bits.

@praveenCY praveenCY force-pushed the topic/add_p6_support branch from 9906eeb to ed1dfa9 Compare July 10, 2021 03:51
@boring-cyborg boring-cyborg bot added the lwip label Jul 10, 2021
@praveenCY praveenCY force-pushed the topic/add_p6_support branch from ed1dfa9 to b64dba1 Compare July 10, 2021 04:04
@bzbarsky-apple bzbarsky-apple requested a review from mspang July 10, 2021 04:20
@praveenCY praveenCY force-pushed the topic/add_p6_support branch 3 times, most recently from 3547693 to e05308f Compare July 14, 2021 05:30
Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

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

Request changes: I believe for build items to be part of the official image, we would wand CHIP developers to be able to validate at least 'this builds' without requiring 3rd party code registration/license agreement/downloads. We would also like to be able to setup CI for our example build targets so they do not break (we are lacking resources right now, but still hoping we can fix or at least run periodically a 'build everything')

For this purpose, all required prerequisites should be accessible at a minimum in the chip-build-vscode. I would start with a PR first that updates the docker images (potentially creating a separate chip-build-infineon image as well for the vscode image to copy from).

@ifyall
Copy link

ifyall commented Jul 22, 2021

@andy31415, @praveenCY has applied the requested changes. Do you have any additional feedback?

Thank you in advance,

Ian

@andy31415 andy31415 dismissed their stale review July 26, 2021 15:35

Removing changes requested: a direct download link for the SDK was provided.

@mspang
Copy link
Contributor

mspang commented Aug 5, 2021

Approved - mostly consensus review though as the change is very large (10K). Assumption is platform-specific files and example app only, so maintenance is defered to infineon

I would like to state for the record, that even if we say that vendors are responsible for their platforms, it is still imposes costs on other contributors. Consider Kevin's error handling work and how adding platform code adds more work to that. (I don't think you are saying that, but what I am saying is that we still need to do a decent review of the code - and we really need to work on the code duplication so that badly need platform interface work does not become impossible to execute).

We have the technical reviewed flag, we can use that here (I can take that for this one). If nobody can review the 10K lines, the patch must not land.

@praveenCY praveenCY force-pushed the topic/add_p6_support branch from 76d4034 to fe3f6ae Compare August 9, 2021 21:34
@praveenCY praveenCY requested a review from mspang August 9, 2021 21:38
@praveenCY praveenCY force-pushed the topic/add_p6_support branch from fe3f6ae to 8d33234 Compare August 10, 2021 07:04
@praveenCY praveenCY force-pushed the topic/add_p6_support branch from 8d33234 to ea8e6bb Compare August 10, 2021 19:05
@mspang
Copy link
Contributor

mspang commented Aug 10, 2021

/gcbrun

@bzbarsky-apple
Copy link
Contributor

@praveenCY praveenCY force-pushed the topic/add_p6_support branch from ea8e6bb to 137fd1f Compare August 11, 2021 22:02
@github-actions
Copy link

Size increase report for "esp32-example-build" from 6a36256

File Section File VM
chip-shell.elf .flash.text 8 8
chip-lock-app.elf .flash.text -68 -68
chip-ipv6only-app.elf .flash.text -172 -172
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.flash.text,8,8
[Unmapped],0,-8

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
[Unmapped],0,68
.flash.text,-68,-68

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize
[Unmapped],0,172
.flash.text,-172,-172


@bzbarsky-apple bzbarsky-apple merged commit 080ae57 into project-chip:master Aug 12, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Add Lock app example for infineon P6 board

* P6 lock app changes addressing review comments

* Update Code changes to support new Clock modifications

* Rename repo to p6_sdk along with docker vscode changes

* Update README.md with dockerfile link for software download

* Remove warnings false check for compile

* Update script to remove sudo and cleanup folder names

* Add all Infineon assets as gitsubmodules

* Address review comments cleaning up code

* Cleanup code as per final review comments

* Address BLE configuration comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants