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

Initial pybind11 Controller Wrapper #8428

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

krypton36
Copy link
Contributor

Problem

  • Switching from ctypes python controller implementation to pybind11.

Change overview

  • Bind Exception error codes.
  • This does not interfere with the current implementation of the python controller.
  • Creates a new directory with the new python controller implementation.
  • Updates to the build_python script to install both wheels created.
  • Updated dev docker file to install python3.9.
  • Added pybind11 git submodule.

Testing

from pybindings.PyChip import ChipExceptions
ChipExceptions.CHIPErrorToException(50)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
pybindings.PyChip.ChipExceptions.ChipErrorTimeout: CHIP_ERROR_TIMEOUT

.devcontainer/Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@mspang mspang left a comment

Choose a reason for hiding this comment

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

The bindings here seem fine to me, thanks for trimming it down.

It might a bit overkill to generate exception types for every error code, but I can see why this might be useful. (e.g. python doesn't do that for 'errno' - OSError is a single type)

scripts/build_python.sh Outdated Show resolved Hide resolved
src/pybindings/pycontroller/BUILD.gn Outdated Show resolved Hide resolved
.devcontainer/Dockerfile Outdated Show resolved Hide resolved
src/pybindings/pycontroller/BUILD.gn Outdated Show resolved Hide resolved
src/pybindings/pycontroller/BUILD.gn Show resolved Hide resolved
src/pybindings/pycontroller/create_error_wrapper.py Outdated Show resolved Hide resolved
src/pybindings/pycontroller/create_error_wrapper.py Outdated Show resolved Hide resolved
src/pybindings/Bnder_Code_Gen.md Outdated Show resolved Hide resolved
@andy31415
Copy link
Contributor

Approved based on offline sync: goal is to wrap minimally-needed (where minimal may not be small) for CSG and we plan to use binder as a starting point, however long term support will involve manually maintaining the files and adding wrappers if needed because of architecture/C++ feature needs.

@mspang
Copy link
Contributor

mspang commented Aug 10, 2021

Some things are still broken about this, such as the PEP 425 tagging and the header paths, but it shouldn't do anyone any harm to fix those issue in a later change.

@woody-apple
Copy link
Contributor

/rebase

@CLAassistant
Copy link

CLAassistant commented Aug 12, 2021

CLA assistant check
All committers have signed the CLA.

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 due to some changes looking like a revert

.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
CHIP_ERROR update from Kevin

Fixes small errors in create_error_wrapper based off the new CHIPError class

Fixing darin workflow to install python3.9

Adding full path to gitmodules
@todo
Copy link

todo bot commented Aug 18, 2021

Log Upload #2227

# TODO Log Upload https://github.com/project-chip/connectedhomeip/issues/2227
# TODO https://github.com/project-chip/connectedhomeip/issues/1512
# - name: Run Code Coverage
# if: ${{ contains('main', env.BUILD_TYPE) }}
# run: scripts/tools/codecoverage.sh
# - name: Upload Code Coverage
# if: ${{ contains('main', env.BUILD_TYPE) }}
# run: bash <(curl -s https://codecov.io/bash)
- name: Remove third_party binaries for CodeQL Analysis
run: find out -type d -name "third_party" -exec rm -rf {} +
- name: Remove dbus binaries for CodeQL Analysis


This comment was generated by todo based on a TODO comment in 9bee5f6 in #8428. cc @krypton36.

@todo
Copy link

todo bot commented Aug 18, 2021

#1512

# TODO https://github.com/project-chip/connectedhomeip/issues/1512
# - name: Run Code Coverage
# if: ${{ contains('main', env.BUILD_TYPE) }}
# run: scripts/tools/codecoverage.sh
# - name: Upload Code Coverage
# if: ${{ contains('main', env.BUILD_TYPE) }}
# run: bash <(curl -s https://codecov.io/bash)
- name: Remove third_party binaries for CodeQL Analysis
run: find out -type d -name "third_party" -exec rm -rf {} +
- name: Remove dbus binaries for CodeQL Analysis
run: find out -type d -name "dbus" -exec rm -rf {} +


This comment was generated by todo based on a TODO comment in 9bee5f6 in #8428. cc @krypton36.

@github-actions
Copy link

Size increase report for "esp32-example-build" from 515ea65

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

Files found only in the build output:
    report.csv

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

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

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

sections,vmsize,filesize

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

sections,vmsize,filesize

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

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

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.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-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

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

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

sections,vmsize,filesize


@woody-apple woody-apple merged commit 1ec6161 into project-chip:master Aug 18, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
CHIP_ERROR update from Kevin

Fixes small errors in create_error_wrapper based off the new CHIPError class

Fixing darin workflow to install python3.9

Adding full path to gitmodules
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