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

Fix ble connect error in Temperature Measurement app #7687

Merged
merged 2 commits into from
Jun 22, 2021

Conversation

PSONALl
Copy link
Contributor

@PSONALl PSONALl commented Jun 16, 2021

Problem

  1. chip-device-ctrl failed to establish a secure session to the device using temperature-measurement-app on mac
  2. resolve command hangs in chip-device-ctrl with temperature-measurement-app

Change overview

  1. Fix ble-connect error in temperature measurement app

    • Added two separate endpoints (Endpoint 0, Endpoint 1) in endpoint_config.h
    • One for Operational Credentials and other for Temperature Measurement
    • Earlier there was only Endpoint 1
  2. Fix resolve mdns hang in temperature measurement app

    • added mdns server start at connectivity change

Testing

  • Tested commissioning with chip-device-ctrl

@dhrishi
Copy link
Contributor

dhrishi commented Jun 16, 2021

@andy31415 @bzbarsky-apple Any idea why the ZAP templates generation has failed?

@woody-apple
Copy link
Contributor

/rebase

@woody-apple woody-apple force-pushed the fix_temperature_app branch from 4c8ba06 to 3cc4719 Compare June 17, 2021 01:45
@PSONALl PSONALl force-pushed the fix_temperature_app branch from 3cc4719 to dba1c67 Compare June 17, 2021 04:55
@PSONALl
Copy link
Contributor Author

PSONALl commented Jun 17, 2021

/rebase

Done!

@bzbarsky-apple
Copy link
Contributor

Any idea why the ZAP templates generation has failed?

Probably because it's catching that endpoint_config.h (a generated file) was hand-edited without actually modifying the data it's generated from, so the next thing to come along and regenerate things will clobber the hand-changes.

Probably what should happen here is changing examples/temperature-measurement-app/esp32/main/temperature-measurement.zap (by hand or via the ZAP ui) to set up the right endpoint configuration, then running ./scripts/tools/zap/generate.py examples/temperature-measurement-app/esp32/main/temperature-measurement.zap to regenerate endpoint_config.h.

@PSONALl PSONALl force-pushed the fix_temperature_app branch from dba1c67 to 5cb5206 Compare June 17, 2021 11:17
@PSONALl
Copy link
Contributor Author

PSONALl commented Jun 17, 2021

Probably what should happen here is changing examples/temperature-measurement-app/esp32/main/temperature-measurement.zap (by hand or via the ZAP ui) to set up the right endpoint configuration, then running ./scripts/tools/zap/generate.py examples/temperature-measurement-app/esp32/main/temperature-measurement.zap to regenerate endpoint_config.h.

Added this change.

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.

Two questions:

  1. Why is this adding Thread Network Diagnostics?
  2. This is putting Temperature Measurement on endpoint 0. What's the point of the (empty, no clusters enabled for it) endpoint 1?

@woody-apple woody-apple self-requested a review June 17, 2021 15:09
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.

Requesting changes per @bzbarsky-apple

@PSONALl
Copy link
Contributor Author

PSONALl commented Jun 18, 2021

Requesting changes per @bzbarsky-apple

Added this change. cc: @bzbarsky-apple @woody-apple

@woody-apple
Copy link
Contributor

@PSONALl PSONALl force-pushed the fix_temperature_app branch from e6080cc to d0f2b1c Compare June 22, 2021 05:18
@PSONALl PSONALl force-pushed the fix_temperature_app branch from d0f2b1c to fd437ab Compare June 22, 2021 05:21
@dhrishi
Copy link
Contributor

dhrishi commented Jun 22, 2021

Approving for the 2nd commit you have added in this PR.

@dhrishi dhrishi self-requested a review June 22, 2021 09:33
@mspang mspang merged commit 90592a7 into project-chip:master Jun 22, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Fix ble connection in Temperature Measurement app

* Fix resolve mdns in temperature measurement app
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.

chip-device-ctrl failed to establish secure session to device using temperature-measurement-app on mac
9 participants