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

Enable arc for chip-device-ctrl #6992

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

vivien-apple
Copy link
Contributor

@vivien-apple vivien-apple commented May 20, 2021

Problem

ARC is not enabled for src/platform/Darwin which results into leaks.

Change overview

  • Enable arc with -fobjc-arc in src/platform/BUILD.gn when chip_device_layer is set to darwin.
  • Enable arc with -fobjc-arc in src/platform/python/BUILD.gn since it contains custom Objc++ files.
  • Update src/platform/Darwin/*.mm files that needs it.
  • Update src/platform/python/chip/darwin/*.mm files.

Testing

This is not especially easy to write tests for ARC. So I did manual testing involving pairing over BLE in order to validate the underlying BLE stack using chip-tool on Mac and iOS ChipTool.

@jmartinez-silabs
Copy link
Member

ARC did find some issues with the Darwin build. Can you push a fix for those build error?

Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

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

Per the updated template, can you update the PR here?

#### Problem
What is being fixed?  Examples:
* Fix crash on startup
* Fixes #12345 12345 Frobnozzle is leaky (exactly like that, so GitHub will auto-close the issue).

#### Change overview
What's in this PR

#### Testing
How was this tested? (at least one bullet point required)
    • If unit tests were added, how do they cover this issue?
    • If unit tests existed, how were they fixed/modified to prevent this in future?
    • If integration tests were added, how do they verify this change?
    • If manually tested, what platforms controller and device platforms were manually tested, and how?
    • If no testing is required, why not?

@vivien-apple vivien-apple requested a review from woody-apple May 21, 2021 17:49
@pullapprove pullapprove bot requested a review from pan-apple May 21, 2021 18:00
@andy31415
Copy link
Contributor

@woody-apple summary was updated according to the template

@stale
Copy link

stale bot commented Jun 1, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Jun 1, 2021
@stale stale bot removed the stale Stale issue or PR label Jun 2, 2021
@woody-apple woody-apple merged commit 5789246 into project-chip:master Jun 3, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
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.

6 participants