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

Sample and command line refactor #394

Merged
merged 34 commits into from
Apr 10, 2023
Merged

Sample and command line refactor #394

merged 34 commits into from
Apr 10, 2023

Conversation

TwistedTwigleg
Copy link
Contributor

@TwistedTwigleg TwistedTwigleg commented Apr 5, 2023

Description of changes:

Refactors samples not to rely on cmdUtils to setup MQTT clients, to connect/disconnect for connect samples, and overall just makes cmdUtils go back to command line parsing and nothing more.

Also contains the following minor changes:

  • Renames CustomKeyOpsPubSub to CustomKeyOpsConnect, removes PubSub part of sample
    • This is because the only difference this sample shows is the connection method, therefore no reason to include the PubSub part.
  • Renames Identity to FleetProvisioning for better visibility. Added note both in README and sample itself that Identity = FleetProvisioning
  • Removed PubSubStress sample entirely
  • Adjusts all samples to register all command line commands and get the data from said commands in a single function, rather than doing it individually in each sample.
    • This also fixed a few minor inconsistencies that slipped through: like ca being used in a couple samples instead of ca_file.
  • Fixes possible Cross-SDK repository communication for PubSub samples
    • Shadow still has potential to communicate across SDKs, as it uses a classic (unnamed) shadow currently.
  • Removed RawConnect sample entirely
    • The AWS IoT MQTT Builder should be used instead. If a sample on using the CRT is wanted/needed, it should be in the CRT repository.
  • Trimmed import calls in all samples to just those that are needed.

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

…Note in a couple places that Identity and Fleet Provisioning are the same to reduce confusion
Copy link
Contributor

@xiazhvera xiazhvera left a comment

Choose a reason for hiding this comment

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

The change looks good to me, and I think this should be the way to go.

While I think Thoai had comments about we should have a more clear main function body. As an example, we can have a static function to contains the build steps in the sample file instead of the util command, so that the main function body only contains the simple function call as before.

It would be good to have more team member look at it.

@TwistedTwigleg
Copy link
Contributor Author

Update 04/07/2023: Removed the RawConnect sample, as we want customers to use the AWS IoT MQTT builders we provide for easy access to AWS. If something like RawConnect is wanted/needed, we should add it to the CRT repository (aws-crt-java).

@TwistedTwigleg
Copy link
Contributor Author

Thanks! Merging into main...

@TwistedTwigleg TwistedTwigleg merged commit de4e5f3 into main Apr 10, 2023
@TwistedTwigleg TwistedTwigleg deleted the sample_refactor branch April 10, 2023 13:08
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.

2 participants