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

Glue standalone application to a device layer for POSIX platforms #1534

Conversation

vivien-apple
Copy link
Contributor

Problem

This patch glue standalone apps to a device layer. Examples of such standalone builds are the one produce by:

  • make -f Makefile-Standalone
  • examples/chip-tool (which use config/standalone)
  • examples/shell (which use config/standalone)

@@ -192,6 +192,14 @@ ifeq ($(ENABLE_CHIPOBLE_TEST),1)
configure_OPTIONS += --enable-chipoble-test=yes
endif

ifeq ($(HOSTOS),darwin)
configure_OPTIONS += --with-device-layer=darwin
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine; however, I'd like to propose auto-detecting this in configure.ac by --with-device-layer supporting and defaulting to an auto value.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will preclude having this logic in both Makefile-Standalone and in standalone-chip.mk.

Copy link
Contributor

Choose a reason for hiding this comment

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

Happy to help with this if you'd like.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I have updated the patch.

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from 8972c7b to d598a6d Compare July 9, 2020 14:55
@vivien-apple vivien-apple requested a review from gerickson July 9, 2020 14:56
@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2020

This pull request introduces 4 alerts and fixes 1 when merging d598a6d into ace290d - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

Copy link
Contributor

@gerickson gerickson left a comment

Choose a reason for hiding this comment

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

Nice work!

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from d598a6d to 4129e4d Compare July 9, 2020 19:24
@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2020

This pull request introduces 4 alerts and fixes 1 when merging 4129e4d into e0f65ef - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch 2 times, most recently from 2542488 to deec3aa Compare July 9, 2020 22:10
@vivien-apple
Copy link
Contributor Author

I have added a few missing headers in src/platform/Makefile.am

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from deec3aa to 6589180 Compare July 9, 2020 22:14
@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch 2 times, most recently from e402b83 to 215ff84 Compare July 9, 2020 22:34
@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2020

This pull request introduces 4 alerts and fixes 1 when merging 215ff84 into e0f65ef - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@woody-apple
Copy link
Contributor

@vivien-apple build failure

@woody-apple woody-apple mentioned this pull request Jul 10, 2020
@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from 215ff84 to b51651a Compare July 10, 2020 16:28
@lgtm-com
Copy link

lgtm-com bot commented Jul 10, 2020

This pull request introduces 4 alerts and fixes 1 when merging 166116a into 18129db - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from 166116a to 8c7ab44 Compare July 10, 2020 17:39
@lgtm-com
Copy link

lgtm-com bot commented Jul 10, 2020

This pull request introduces 4 alerts and fixes 1 when merging 8c7ab44 into 18129db - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from 8c7ab44 to c252466 Compare July 10, 2020 18:21
@lgtm-com
Copy link

lgtm-com bot commented Jul 10, 2020

This pull request introduces 4 alerts and fixes 1 when merging c252466 into 0c1fc15 - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@github-actions
Copy link

Size increase report for "linux-example-build"

File Section File VM
chip-standalone-demo.out [LOAD #5 [RW]] 0 16
chip-standalone-demo.out .eh_frame -8 -8
chip-standalone-demo.out .data -16 -16
chip-standalone-demo.out .text -32 -32
chip-standalone-demo.out .bss 0 -14080
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-standalone-demo.out and ./pull_artifact/chip-standalone-demo.out:

sections,vmsize,filesize
.debug_line,0,3512
.debug_str,0,625
.debug_macro,0,287
[Unmapped],0,37
[LOAD #5 [RW]],16,0
.eh_frame,-8,-8
.data,-16,-16
.symtab,0,-24
.strtab,0,-31
.text,-32,-32
.debug_loc,0,-126
.debug_abbrev,0,-359
.debug_info,0,-513
.bss,-14080,0


@andy31415
Copy link
Contributor

This seems to be the first version that has a diffable bloat report.

Is 14K really correct? Is the minus correct? I see only addition - I wonder if our bot does things backwards.

@vivien-apple - do you think a large BSS size change is expected from this PR? Is a 'decrease' correct?

@woody-apple
Copy link
Contributor

@vivien-apple Seems to be breaking builds still :(

@vivien-apple
Copy link
Contributor Author

@vivien-apple Seems to be breaking builds still :(

Sorry about that and my also for the way I have used the CI.
There are some differences between the CI and the VSCode container when it comes to build the dist package related to the ot-br-posix dependency.

I finally found a way to get the build system happy locally but I will open a specific bug for this issue as it is unclear to me what it works on CI and not on VSCode.

@vivien-apple
Copy link
Contributor Author

This seems to be the first version that has a diffable bloat report.

Is 14K really correct? Is the minus correct? I see only addition - I wonder if our bot does things backwards.

@vivien-apple - do you think a large BSS size change is expected from this PR? Is a 'decrease' correct?

It was not expected. But the configuration of the demo build may have changed with the platform config flags in src/platform and something could have been disabled.

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch 2 times, most recently from c252466 to a43f6d6 Compare July 15, 2020 16:56
@lgtm-com
Copy link

lgtm-com bot commented Jul 15, 2020

This pull request introduces 4 alerts and fixes 1 when merging 8f0b23c into bca8b6e - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from 8f0b23c to 98632e5 Compare July 15, 2020 17:22
@lgtm-com
Copy link

lgtm-com bot commented Jul 15, 2020

This pull request introduces 4 alerts and fixes 1 when merging 98632e5 into bca8b6e - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from 98632e5 to 65597f6 Compare July 15, 2020 18:46
@github-actions
Copy link

Size increase report for "nrf-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-nrf52840-lock-example.out and ./pull_artifact/chip-nrf52840-lock-example.out:

sections,vmsize,filesize


@lgtm-com
Copy link

lgtm-com bot commented Jul 15, 2020

This pull request introduces 4 alerts and fixes 1 when merging 65597f6 into bca8b6e - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@vivien-apple vivien-apple force-pushed the Darwin_GlueStandaloneAppToDeviceLayer branch from 65597f6 to aedca64 Compare July 16, 2020 10:26
@github-actions
Copy link

Size increase report for "esp32-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-wifi-echo.elf and ./pull_artifact/chip-wifi-echo.elf:

sections,vmsize,filesize


@github-actions
Copy link

Size increase report for "gn_nrf-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@github-actions
Copy link

Size increase report for "gn_linux-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2020

This pull request introduces 4 alerts and fixes 1 when merging aedca64 into bca8b6e - view on LGTM.com

new alerts:

  • 4 for Expression has no effect

fixed alerts:

  • 1 for Expression has no effect

@andy31415 andy31415 merged commit 694f20d into project-chip:master Jul 16, 2020
kedars pushed a commit to kedars/connectedhomeip that referenced this pull request Jul 21, 2020
kedars pushed a commit to kedars/connectedhomeip that referenced this pull request Jul 21, 2020
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request Feb 15, 2024
…p when it is uncommissioned

Merge in WMN_TOOLS/matter from feature/917_sleepy_uncommissioned to RC_2.3.0-1.3

Squashed commit of the following:

commit 0d5350c7dc0468aeb1c7bcf7f8cb8833b8e1427a
Author: chbansal <chirag.bansal@silabs.com>
Date:   Thu Feb 8 15:07:37 2024 +0530

    reverting the DIC_ENABLE macro to back place

commit 5df0f689f39525889979e6e609f56b463611c3dc
Author: chbansal <chirag.bansal@silabs.com>
Date:   Wed Feb 7 23:15:22 2024 +0530

    Adding different Appdelegate to not allow the DUT go to sleep in between commissioning

commit 4bfd3a125ec5561325ee29b263479821253cc69d
Author: chbansal <chirag.bansal@silabs.com>
Date:   Wed Feb 7 21:43:01 2024 +0530

    reverting the BLEManagerImpl.cpp

... and 7 more commits
chirag-silabs added a commit to rosahay-silabs/connectedhomeip that referenced this pull request Feb 19, 2024
…p when it is uncommissioned

Merge in WMN_TOOLS/matter from feature/917_sleepy_uncommissioned to RC_2.3.0-1.3

Squashed commit of the following:

commit 0d5350c7dc0468aeb1c7bcf7f8cb8833b8e1427a
Author: chbansal <chirag.bansal@silabs.com>
Date:   Thu Feb 8 15:07:37 2024 +0530

    reverting the DIC_ENABLE macro to back place

commit 5df0f689f39525889979e6e609f56b463611c3dc
Author: chbansal <chirag.bansal@silabs.com>
Date:   Wed Feb 7 23:15:22 2024 +0530

    Adding different Appdelegate to not allow the DUT go to sleep in between commissioning

commit 4bfd3a125ec5561325ee29b263479821253cc69d
Author: chbansal <chirag.bansal@silabs.com>
Date:   Wed Feb 7 21:43:01 2024 +0530

    reverting the BLEManagerImpl.cpp

... and 7 more commits
rcasallas-silabs pushed a commit to rcasallas-silabs/connectedhomeip that referenced this pull request Jun 20, 2024
…p when it is uncommissioned

Merge in WMN_TOOLS/matter from feature/917_sleepy_uncommissioned to RC_2.3.0-1.3

Squashed commit of the following:

commit 0d5350c7dc0468aeb1c7bcf7f8cb8833b8e1427a
Author: chbansal <chirag.bansal@silabs.com>
Date:   Thu Feb 8 15:07:37 2024 +0530

    reverting the DIC_ENABLE macro to back place

commit 5df0f689f39525889979e6e609f56b463611c3dc
Author: chbansal <chirag.bansal@silabs.com>
Date:   Wed Feb 7 23:15:22 2024 +0530

    Adding different Appdelegate to not allow the DUT go to sleep in between commissioning

commit 4bfd3a125ec5561325ee29b263479821253cc69d
Author: chbansal <chirag.bansal@silabs.com>
Date:   Wed Feb 7 21:43:01 2024 +0530

    reverting the BLEManagerImpl.cpp

... and 7 more commits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants