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.

Sign up for GitHub

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

Jump to bottom

Add MqttClientConnection as an interface for usage in IoTJobsClient #207

Closed
wants to merge 8 commits into from
Closed

Add MqttClientConnection as an interface for usage in IoTJobsClient #207

wants to merge 8 commits into from

Conversation

MikeDombo
Copy link

@MikeDombo MikeDombo commented Jun 25, 2020 •
edited
Loading

Issue #, if available:

Description of changes:
This change creates the interfaces MqttPublishInterface, MqttSubscribeInterface, and MqttSubscribeHandlerInterface (if you have a preferred name, let me know and I'll change it) which are implemented by MqttClientConnection. The purpose of this change is to enable a change to the IotJobsClient to accept the interface instead of the concrete implementation.

Should this PR be accepted, it should be followed by a change in AWS-IoT-Device-SDK-v2 for the IotJobsClient to use this added interface instead of the concrete implementation.

This PR would resolve #124.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

bretambrose
bretambrose requested changes Jul 2, 2020
View reviewed changes
Copy link
Contributor

@bretambrose bretambrose left a comment

Choose a reason for hiding this comment

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

I think this is a good direction. Couple changes I'd like to see, I'll also try and get some others to take a look.

  1. I think we need to also add the handler-less subscribe. I've been told that future IoT Apis may use that function.
  2. I'd like to rename the interface to something that better captures what this is a proxy for (not an entire mqtt connection, but the data pipeline/pubsub part). I'm not sure what a good name is, MqttDataAdapter, MqttPubSubAdapter, etc... Proxy captures it decently, but I think that would be confusing with its other uses. Maybe even MqttDataInterface.

@MikeDombo
Copy link
Author

MikeDombo commented Jul 2, 2020 •
edited
Loading

Hi @bretambrose, thanks for the review.

What would you think about having maybe 3 interfaces so that the various use cases can have minimal requirements on the client they are given?

1 interface for pubsub with handler, 1 for pubsub without handler and with global handler, and then 1 which is the same as the concrete implementation?

@MikeDombo MikeDombo requested a review from bretambrose July 13, 2020 15:28
@justinboswell justinboswell changed the base branch from master to main November 13, 2020 02:57
cmello pushed a commit that referenced this pull request Dec 9, 2021
CRT submodules update
148a0c8 
aws-c-auth     v0.6.8 ✓
aws-c-cal     v0.5.12 ✓
aws-c-common    v0.6.17 -> v0.6.18
aws-c-compression v0.2.14 ✓
aws-c-event-stream v0.2.7 ✓
aws-c-http     v0.6.10 ✓
aws-c-io      v0.10.13 -> v0.10.14
aws-c-mqtt     v0.7.9 -> v0.7.10
aws-c-s3      v0.1.27 -> v0.1.30
aws-c-sdkutils   v0.1.1 ✓
aws-checksums   v0.1.12 ✓
aws-lc       v0.0.2 ✓
s2n        v1.3.0 -> v1.3.1

Diff:

Submodule crt/aws-c-common 4639b27..cba2308:
  > Support for Windows server 2008 (#868)
  > Bump Litani version to 1.16.0 (#872)
  > Upgrade CBMC submodules (#871)
  > Remove aws_ring_buffer_allocator_init declaration from public header (#870)
  > Make byte_buf proof allocator consistent with code (#869)
  > Removes unused stubs from CBMC proofs (#867)
Submodule crt/aws-c-io da95b34..cb9dcea:
  > const pointer (#456)
  > Set default cpu_id on thread_options (#455)
  > Windows header (#454)
  > Improved error reporting when getaddrinfo() fails (#452)
Submodule crt/aws-c-mqtt 60f9a17..6168e32:
  > remove try lock from debug build (#207)
Submodule crt/aws-c-s3 8655417..76f49fb:
  > Enable headers callback invocation for failed requests (#162)
  > remove try lock from debug build (#161)
  > [dashboard] update dependencies and tirvial bug fix (#160)
  > fix build for cross compile (#157)
  > Add missing dependency in README.md (#156)
  > Remove package locks irrelevant to aws-c-s3's security stance (#155)
  > Remove package locks irrelevant to aws-c-s3's security stance (#154)
Submodule crt/s2n ab9a3be..7d02417:
  > Nitpick usage guide links (#3133)
  > FIPS Static Config is Only Created When Needed (#3129)
  > Fix build on NetBSD. (#3131)
  > Feature probe for EVP_md5_sha1() (#3128)
  > Allow EVP hash implementation to use EVP_md5_sha1 if available (#3126)
  > Allow synchronous private key operations (#3121)
@cmello cmello mentioned this pull request Dec 9, 2021
Merged
cmello pushed a commit that referenced this pull request Dec 10, 2021
CRT submodules update (#421)
7116d27 
* CRT submodules update

aws-c-auth     v0.6.8 ✓
aws-c-cal     v0.5.12 ✓
aws-c-common    v0.6.17 -> v0.6.18
aws-c-compression v0.2.14 ✓
aws-c-event-stream v0.2.7 ✓
aws-c-http     v0.6.10 ✓
aws-c-io      v0.10.13 -> v0.10.14
aws-c-mqtt     v0.7.9 -> v0.7.10
aws-c-s3      v0.1.27 -> v0.1.30
aws-c-sdkutils   v0.1.1 ✓
aws-checksums   v0.1.12 ✓
aws-lc       v0.0.2 ✓
s2n        v1.3.0 -> v1.3.1

Diff:

Submodule crt/aws-c-common 4639b27..cba2308:
  > Support for Windows server 2008 (#868)
  > Bump Litani version to 1.16.0 (#872)
  > Upgrade CBMC submodules (#871)
  > Remove aws_ring_buffer_allocator_init declaration from public header (#870)
  > Make byte_buf proof allocator consistent with code (#869)
  > Removes unused stubs from CBMC proofs (#867)
Submodule crt/aws-c-io da95b34..cb9dcea:
  > const pointer (#456)
  > Set default cpu_id on thread_options (#455)
  > Windows header (#454)
  > Improved error reporting when getaddrinfo() fails (#452)
Submodule crt/aws-c-mqtt 60f9a17..6168e32:
  > remove try lock from debug build (#207)
Submodule crt/aws-c-s3 8655417..76f49fb:
  > Enable headers callback invocation for failed requests (#162)
  > remove try lock from debug build (#161)
  > [dashboard] update dependencies and tirvial bug fix (#160)
  > fix build for cross compile (#157)
  > Add missing dependency in README.md (#156)
  > Remove package locks irrelevant to aws-c-s3's security stance (#155)
  > Remove package locks irrelevant to aws-c-s3's security stance (#154)
Submodule crt/s2n ab9a3be..7d02417:
  > Nitpick usage guide links (#3133)
  > FIPS Static Config is Only Created When Needed (#3129)
  > Fix build on NetBSD. (#3131)
  > Feature probe for EVP_md5_sha1() (#3128)
  > Allow EVP hash implementation to use EVP_md5_sha1 if available (#3126)
  > Allow synchronous private key operations (#3121)

* Reverted S2N to Release v1.3.0
@MikeDombo MikeDombo closed this by deleting the head repository Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bretambrose bretambrose Awaiting requested review from bretambrose

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add interfaces to make it possible to use different MQTT clients
2 participants
@MikeDombo @bretambrose