aws / aws-iot-device-sdk-js-v2

Next generation AWS IoT Client SDK for Node.js using the AWS Common Runtime
Apache License 2.0
220 stars 99 forks source link

Overly complicated api #25

Closed mattsoftware closed 2 years ago

mattsoftware commented 4 years ago

The demo mqtt connection did not work for me. After trying to work out how to connect to aws iot using the demo provided left me scratching my head wondering how to do it it. After I spent 20 minutes on it with no luck I went to the old sdk. 2 minutes and 20 lines of javascript later I had a working mqtt test app with no issues.

I have to say the event driven architecture the old sdk seems quite intuitive, reliable and so simple that I find it hard to understand why it needs to be replaced with a complete rewrite.

Is there any chance to keep some of the event driven design from the old api in this model too? All I really need from this sdk is to give it some certificate info, a device name, an iot endpoint, and then some event handlers. (I understand thats not everyones objective, but having an overly complicated api to replace a simple working one seems a bit bizarre to me.)

JonathanHenson commented 4 years ago

Could you provide some particular examples about what you’d like to keep from the v1 sdk? For example, the event driven ness is still very much there in v2, so I’m assuming you mean something else.

What are some things we could simplify for you here?

mattsoftware commented 4 years ago

Maybe this is a lack of documentation issue. It may also be that I prefer javascript to typescript. I am taking my documentation from the sample code in v2 (https://github.com/aws/aws-iot-device-sdk-js-v2/blob/master/samples/node/pub_sub/index.ts).

It seems to attach to the broker I need to...

Oh, It does look like dealing with the mqtt client looks familiar, maybe I missed that because I was not able to get the above steps working correctly. All of the above I replaced in my code with these lines from v1...

const awsIot = require('aws-iot-device-sdk'); const device = awsIot.device({ keyPath: 'cert.key', certPath: 'cert.pem', caPath: 'root.ca.pem', clientId: 'monitor', host: '...', });

After getting the device object I can now put event listeners on it.

Which in my opinion is way easier than all the steps I could find for v2 (that didn't end up working for me). Hopefully this is simply a documentation issue. If there is a similar way to do this in V2 I would appreciate to know how so I can start using it (to be honest having the greengrass discovery built into the device sdk would be pretty sweet if I can get that working :)

Thanks for your reply

JonathanHenson commented 4 years ago

No doubt v1 is easier. V2 is design for long term growth (plus some other design concerns) and follows unix design principles closely. That’s often a trade off from easy to simple which are often concerns that are at odds with one another. That said, I’d love to create some optional easier to use interfaces and more thorough documentation is in the works.

The problem is that the easier interface will be prone to subtle programming errors (global state on our side or the accidental creation of more threads than you need on yours). If you manage to get it working for you, I’d love some ideas on how you’d improve the interface given these concerns.

JonathanHenson commented 4 years ago

I’d like to add, the things vanilla nodejs does Implicitly for you are very similar to the apis we force you to configure explicitly here. The idea is to remove implicit behavior altogether and to give you more explicit control over your configuration. I’d like a way to balance these concerns in a easy api (sort of like the way Libcurl has a easy api and a “trust me I’m a programmer api). I’m open to any ideas here on an easy api that isn’t prone to accidental bad decisions.

For example, node doesn’t give you control over TLS or event loop threads. We chose to put those decisions into the hands of the user. I think this is an all around better interface. It’s also a lot more boiler plate setup code. How can we resolve these two things into a single interface?

Doff3n commented 4 years ago

+1 on this, I find a lack of documentation and keep scratching my head implementing fleet provisioning.

I find the sample to have quite a lot of implementation details: yargs, bootstraping, main function, configuration parameteres etc. Some simple documentation for how to implement fleet provisioning just using the API would help, but instead a lot of your issue responses and your readme is referencing the samples.

An example of documentation could be something like the one below.


Fleet provisioning:

Prerequisite

You need to set up a fleet provision template in the aws console.

Quickstart

The certificate request is transferred over mqtt(?).

  1. install the SDK

npm install aws-iot-device-sdk-js-v2

  1. main flow:
    //iotidentity is used for certificate creation and registering things(?)
    import iotidentity from 'aws-iot-device-sdk-js-v2'
    //creates the sertificate
    iotidentity.publishCreateKeysAndCertificate(input a, input b)
    //creates the thing
    iotidentity.publishRegisterThing(input a, input b, input c)

input b: QoS describes levels of QoS in the MQTT message.

Sample

bretambrose commented 4 years ago

I really like your template and I agree about the confusion with respect to the provisioning sample. I struggled for half a day to figure out what I needed to do get the sample to work as the IoT service docs were unclear and left some significant steps out. What helped me most was https://docs.aws.amazon.com/freertos/latest/lib-ref/c-sdk/provisioning/provisioning_tests.html#provisioning_system_tests_setup which is a very detailed version of part of what you're suggesting.

I feel that your concern is separate from this existing issue and is more a "for each sample, add a readme that gives explicit step-by-step instructions to go from nothing to a working sample" Could you create a separate issue for this?

bretambrose commented 4 years ago

Also, per the original issue, I wonder if adding some "ezDefault" construction options to the builder that literally mimic the old device constructor APIs and completely hide the additional controls (bootstrap, event loop group, etc...) would be a good bridge solution between v1 and v2.

Doff3n commented 4 years ago

In addition to bridging the solution between v1 and v2 I would say that the methods execute_keys_session and execute_csr_session could be contained in the library.

I think that a cleaner API would be CreateKeysAndCertificate and RegisterThing with some input taking in either an initial CA CSR or certificate/keys. I am not sure that makes sense?

I created a seperate issue where I request more documentation #66.

a428tm commented 4 years ago

Is there a reason why it is written in Typescript?

ZaneL commented 4 years ago

I agree with this. I've avoided using this library because the examples are bad and the API seems overly-complex.

DusanBrejka commented 3 years ago

The primary purpose of any AWS SDK should be making the use of APIs easier. This abominable SDK is literally doing the opposite.

We'll be sticking to v1 on all of our current and future projects.

eduardosanzb commented 3 years ago

Could you please attach samples of how to use your code?

BillClarkDemTech commented 3 years ago

I don't know that the problem with the API is complexity; that's ok if what is being abstracted is itself complex. However, what makes the API unusable for me is the lack of usable documentation. My needs are extremely simple: I want to publish a message from a device. That's it. For the life of me I cannot figure out how to do it using this API. A simple example from start to finish using js, not ts, would solve my problem.

tporter-signiant commented 3 years ago

Any update on coming documentation improvements for V2? I'm in the process of trying to migrate a project from V1 to V2 in order to resolve some stability issues we're have with a V1 client under heavier message load but I'm finding the lack of documentation making it more difficult than it should be beyond the basics that are shown in the current samples. Seems that if V2 is going to be promoted as the SDK of choice the documentation should be at least as comprehensive as the previous version. Most concerning is that there doesn't appear to be much change to the docs in the last year.

Was specifically looking for docs on what events are emitted by MqttClientConnection objects without having to comb through source code to figure it out. Previously one could subscribe to events such as connect, reconnect, error, disconnect and offline but it's not obvious in V2.

jmklix commented 3 years ago

Thanks for the feedback, we're looking into ways to improve this sdk.

@tporter-signiant as for your specific question you can find code here.

imZack commented 3 years ago

It's painful to use the v2 SDK to achieve any simple tasks. I'm also looking for events emitted by MqttClientConnection and it's not easy to navigate between different Repos, native code, TypeScript.

georgeinggs commented 2 years ago

I agree. We use multiple AWS services without any real difficulties and now that we've come across a real-world use case for IoT, I've just about given up after half a day of trying to get even a basic connection up and running. Ultimately I've bitten the bullet, installed V1 and within 15 minutes had a working connection with pub/sub doing exactly what's advertised on the box.

Personally I think the V2 API is MASSIVELY over-engineered for what is actually meant to be a very light and simple task, particularly when it's being deployed and run on very low-level, low-performance devices.

Maybe with some more clear documentation and examples V2 could be useful, but we'll stick with V1 for now I think.

steffenstolze commented 2 years ago

The documentation is really lacking simple examples and instructions where to begin.

In case anyone is searching for a How do I subscribe to a topic? example, this should help you:

Simple Example using client certificate

Since the official documentation of https://github.com/aws/aws-iot-device-sdk-js-v2 is lacking simple examples, I tried to figure it out by myself and came up with a few lines of code, allowing me to subscribe to a specific topic and print out received messages.

Prerequisites

Those instructions are working for macOS.

Before installing the AWS IoT Device SDK, you need to download / install CMake: https://cmake.org/download/

The official AWS IoT Device SDK Documentation gives additional instructions if you want / can use apt-get or yum. I just used the installer and added it to the path:

sudo /Applications/CMake.app/Contents/bin/cmake-gui --install=/usr/local/bin

then install the AWS IoT Device SDK

npm install aws-iot-device-sdk-v2 --save

create an index.js file and a certs folder and copy the device.crt, device.key and ca.pem files (e.g., from the AWS console) into it.

Code

I also provided the links to the specific API Documentation sections for each step.

Paste the following code into index.jsfile:

const awsiot = require('aws-iot-device-sdk-v2')
const TextDecoder = require('util').TextDecoder;

const iot = awsiot.iot
const mqtt = awsiot.mqtt
const decoder = new TextDecoder('utf8')

//https://aws.github.io/aws-iot-device-sdk-js-v2/classes/aws_iot.awsiotmqttconnectionconfigbuilder.html
let configBuilder = iot.AwsIotMqttConnectionConfigBuilder.new_mtls_builder_from_path('./certs/device.crt', './certs/device.key')
configBuilder.with_certificate_authority_from_path(undefined, './certs/AmazonRootCA1.pem')
configBuilder.with_clean_session(true);
configBuilder.with_client_id("test-" + Math.floor(Math.random() * 100000000));
configBuilder.with_endpoint('xxxxxxxxxxxxxx-ats.iot.eu-central-1.amazonaws.com');

const config = configBuilder.build();

//https://aws.github.io/aws-iot-device-sdk-js-v2/classes/mqtt.mqttclient.html
const client = new mqtt.MqttClient();

//https://aws.github.io/aws-iot-device-sdk-js-v2/classes/mqtt.mqttclient.html#new_connection
const connection = client.new_connection(config);

//https://aws.github.io/aws-iot-device-sdk-js-v2/classes/mqtt.mqttclientconnection.html#connect
connection.connect()

//https://aws.github.io/aws-iot-device-sdk-js-v2/classes/mqtt.mqttclientconnection.html#subscribe
connection.subscribe('test', 0)

//https://aws.github.io/aws-iot-device-sdk-js-v2/modules/mqtt.html#mqttconnectionconnected
connection.on('connect', sessionPresent => {
    console.log('Connected.', sessionPresent)
})

//https://aws.github.io/aws-iot-device-sdk-js-v2/modules/mqtt.html#mqttconnectiondisconnected
connection.on('disconnect', () => {
    console.log('Disconnected.')
})

//https://aws.github.io/aws-iot-device-sdk-js-v2/modules/mqtt.html#mqttconnectionerror
connection.on('error', err => {
    console.log('Error:', err)
})

//https://aws.github.io/aws-iot-device-sdk-js-v2/modules/mqtt.html#onmessagecallback
connection.on('message', (topic, payload, dup, qos, retain) => {
    console.log(`Received message on topic '${topic}':`)
    console.log(decoder.decode(payload))
    console.log('DUP:', dup)
    console.log('QoS:', qos)
    console.log('Retain:', retain)
})

Success.

TwistedTwigleg commented 2 years ago

Thank you to everyone who has participated in this issue. There is a lot of information here and we appreciate everyone putting in the time to leave us feedback.

Many of the issues brought up have been fixed:

For the remaining issues, we are continuing to work on them and are taking all feedback under consideration. Thank you again for everyone who has participated and left feedback.

I am closing this issue. If you find problems with the latest version of the SDK, please make a new GitHub issue detailing the problem so we can take a look and work to find a solution. Thanks

github-actions[bot] commented 2 years ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.