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

Rename CHIPConnection to CHIPUdpExchange #711

Closed

Conversation

andy31415
Copy link
Contributor

Reasoning is that UDP is connectionless, so we use "exchange" to denote
exchanging messages.

Future CHIPConnection will actually handle connection protocols (tcp,
ble).

Problem

Short version is "UDP is connectionless".
We are considering that several transports including UDP, TCP, BLE, ... are ok for exchanging requests and replies between devices, however a "connection" has additional meaning.

Summary of Changes

Renames existing CHIPConnection to ChipUdpExchange.
Weave port of CHIPConnection does the connection bit (TCP and BLE) and will not replace this class. The weave message layer does support UDP however that will only come in once we have the exchange bits ported. Only at that time, an code merge & update can be feasable.

working towards #157 and #158.

Reasoning is that UDP is connectionless, so we use "exchange" to denote
exchanging messages.

Future CHIPConnection will actually handle connection protocols (tcp,
ble).
@codecov
Copy link

codecov bot commented May 14, 2020

Codecov Report

Merging #711 into master will increase coverage by 5.72%.
The diff coverage is 52.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #711      +/-   ##
==========================================
+ Coverage   53.93%   59.66%   +5.72%     
==========================================
  Files         135      130       -5     
  Lines       13654    12650    -1004     
==========================================
+ Hits         7364     7547     +183     
+ Misses       6290     5103    -1187     
Impacted Files Coverage Δ
src/ble/BLEEndPoint.cpp 0.00% <ø> (ø)
src/ble/BleError.cpp 99.04% <ø> (+99.04%) ⬆️
src/ble/BleLayer.cpp 0.00% <ø> (ø)
src/ble/BtpEngine.cpp 0.00% <ø> (ø)
src/ble/BtpEngine.h 0.00% <ø> (ø)
src/controller/CHIPDeviceController.cpp 0.00% <0.00%> (ø)
src/crypto/CHIPCryptoPALOpenSSL.cpp 97.83% <ø> (ø)
src/crypto/CHIPOpenSSL.c 96.65% <ø> (ø)
src/inet/AsyncDNSResolverSockets.cpp 0.00% <ø> (ø)
src/inet/DNSResolver.cpp 0.00% <ø> (ø)
... and 85 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96cde96...11dc03d. Read the comment docs.

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.

Seems innocuous if it supports a larger, later goal.

@sagar-apple
Copy link
Contributor

Rename CHIPConnection to CHIPUdpExchange.h

Nit: the PR title doesn't need to specify the .h

@sagar-apple
Copy link
Contributor

LGTM!

@andy31415, how do you feel about filing an issue for this. Just so we never forget to come back and clean/reorg ChipUdpExchange. Maybe when Message Layer comes in this becomes obsolete ?

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.

I thought per discussion we'd do this when the CHIPConnection + Managers above are in place, that would replace this?

Let's do this as a part of: #157

@andy31415 andy31415 changed the title Rename CHIPConnection to CHIPUdpExchange.h. Rename CHIPConnection to CHIPUdpExchange May 20, 2020
@andy31415
Copy link
Contributor Author

I thought per discussion we'd do this when the CHIPConnection + Managers above are in place, that would replace this?

Let's do this as a part of: #157

I am trying to make deltas smaller. This rename can for sure be part of a WML move, however it seems that renaming is very self-contained so I figured we can approve separately.

Please LMK what to do with this - there are a few approvals here, however I would only like to submit when there are no outstanding comments.

@woody-apple
Copy link
Contributor

woody-apple commented May 21, 2020

I thought per discussion we'd do this when the CHIPConnection + Managers above are in place, that would replace this?
Let's do this as a part of: #157

I am trying to make deltas smaller. This rename can for sure be part of a WML move, however it seems that renaming is very self-contained so I figured we can approve separately.

Please LMK what to do with this - there are a few approvals here, however I would only like to submit when there are no outstanding comments.

Can we do this as part of the other discussion draft PR? I think this helps bring context to that change.

@andy31415

@andy31415
Copy link
Contributor Author

I thought per discussion we'd do this when the CHIPConnection + Managers above are in place, that would replace this?
Let's do this as a part of: #157

I am trying to make deltas smaller. This rename can for sure be part of a WML move, however it seems that renaming is very self-contained so I figured we can approve separately.
Please LMK what to do with this - there are a few approvals here, however I would only like to submit when there are no outstanding comments.

Can we do this as part of the other discussion draft PR? I think this helps bring context to that change.

@andy31415

I created a wholesale PR with the entire message layer in #756

It is huge and I am still unclear what was agreed upon regardig the messaging layer. I get conflicting information. What would be the best way forward?

@andy31415
Copy link
Contributor Author

Closing as we will move these into the transpor layer instead. I will rename them in that context.

@andy31415 andy31415 closed this May 29, 2020
kvikrambhat added a commit to kvikrambhat/connectedhomeip that referenced this pull request Feb 24, 2023
kvikrambhat added a commit to kvikrambhat/connectedhomeip that referenced this pull request Feb 24, 2023
bzbarsky-apple pushed a commit that referenced this pull request Feb 24, 2023
* Update Test_TC_ACL_2_1.yaml

Fixes Issues #711[CHIP-Specifications/chip-certification-tool#711]

* Updating auto gen files

* undo formatting changes
lecndav pushed a commit to lecndav/connectedhomeip that referenced this pull request Mar 22, 2023
* Update Test_TC_ACL_2_1.yaml

Fixes Issues project-chip#711[CHIP-Specifications/chip-certification-tool#711]

* Updating auto gen files

* undo formatting changes
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request May 2, 2023
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request May 18, 2023
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
rerasool pushed a commit to SiliconLabs/matter that referenced this pull request Jun 23, 2023
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request Sep 1, 2023
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
shgutte pushed a commit to shgutte/connectedhomeip that referenced this pull request Oct 5, 2023
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request Oct 25, 2023
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
shgutte pushed a commit to shgutte/connectedhomeip that referenced this pull request Jan 11, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request Jan 18, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Jan 29, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Jan 29, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request Feb 15, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request May 8, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
rcasallas-silabs pushed a commit to rcasallas-silabs/connectedhomeip that referenced this pull request Jun 20, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
rcasallas-silabs pushed a commit to rcasallas-silabs/connectedhomeip that referenced this pull request Jun 20, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
chirag-silabs pushed a commit to rosahay-silabs/connectedhomeip that referenced this pull request Jul 15, 2024
…e + Nest Hub

Merge in WMN_TOOLS/matter from feature/unifymatterbridge-googlehome to silabs

Squashed commit of the following:

commit bcadf935457d1e8b5d7a5b9d133a03e24a7b9ef6
Author: Jonas Roum-Møller <[email protected]>
Date:   Mon Apr 24 13:47:43 2023 +0200

    Added steps for configuring Google Home + Nest Hub
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.

8 participants