Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

Adapt to production MQTT protocol #1

Closed
johanstokking opened this issue Sep 1, 2016 · 6 comments
Closed

Adapt to production MQTT protocol #1

johanstokking opened this issue Sep 1, 2016 · 6 comments
Assignees

Comments

@johanstokking
Copy link
Contributor

See https://github.com/TheThingsNetwork/ttn/tree/refactor/mqtt

@FokkeZB
Copy link
Contributor

FokkeZB commented Sep 1, 2016

@johanstokking, @htdvisser may I have your input on these changes for node-ttn to work with production?

Changes in the message formats:

  • downlink
    • payload > payload_raw
    • no more ttl
  • uplink
    • fields > payload_fields
    • payload > payload_raw
    • Include full metadata array, not just first object in array
    • Inject Dev EUI as dev_eui instead of devEUI, like MQTT does in activations
  • activation
    • Pass on full message instead of just devEUI, which also means this is now dev_eui.

Changes in the broker address

  • If staging.thethingsnetwork.org is give, enable legacy flag so the library continuous to work with those message formats
  • If we will have a default production broker, make that constructor argument optional.
  • Allow the user to just give the region and compose the broker URL in the constructor.

Other improvements while we're at it:

  • connect
    • Pass along connack argument?
  • end
    • Send along the optional force and cb options?

@htdvisser
Copy link
Contributor

The URLs for the Load Balancers of our datacenters are currently as follows:

  • eu.thethings.network
  • us-west.thethings.network
  • brazil.thethings.network
  • se-asia.thethings.network

We might want to create additional CNAMES for mqtt/router/broker/handler, but maybe the plain URLs are more clear?

@FokkeZB
Copy link
Contributor

FokkeZB commented Sep 2, 2016

Thanks @htdvisser, I'd suggest that the Client constructor will accept:

  • 'eu.thethings.network', 'app id', 'app key'
  • 'eu', 'app id', 'app key': when the first arg doesn't contain a . it will add .thethings.network
  • 'app id', 'app key': when the first arg is skipped it defaults to eu.thethings.network

We just need to be sure that the we never introduce something like us.west but always stick to dashes, just like Amazon does.

@htdvisser
Copy link
Contributor

I think defaulting to eu (last bullet) is a bad idea. Applications can only be registered with one handler, and therefore messages will only arrive on one MQTT broker. Users should explicitly pass the region.

@FokkeZB
Copy link
Contributor

FokkeZB commented Sep 2, 2016

Clear, thx!

@FokkeZB
Copy link
Contributor

FokkeZB commented Sep 6, 2016

As part of the refactor I've simplified the API

https://github.com/TheThingsNetwork/node-ttn/blob/refactor/src/example.js

  • Client(broker, appEUI, accessKey) > Client(region, appId, accessKey)
  • uplink -> message
  • downlink() -> send()

For discussion on standardising on a simple API for SDKs:

https://docs.google.com/document/d/1nk6_Tty1WZ3RCl0hOVspLq6h87ZrQQiL2OjFguNdK7s/edit?usp=sharing

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants