Mqtt5

[Steve-Mcl]

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Proposed changes

Support major features of MQTT V5

Checklist

• I have read the contribution guidelines
• For non-bugfix PRs, I have discussed this change on the forum/slack team.
• I have run grunt to verify the unit tests pass
• I have added suitable unit tests to cover the new/changed functionality

Details

Summary (from forum thread)

Server/Broker options	Implementation notes	Testing	Follow up
Session expiration. Split the clean-up session flag into a new start flag and the session expiration interval can be modified when disconnected.	number type input. Type checked and coerced into integer before updating v5 properties object	100%
Flow control. Allow the client and server to specify the number of outstanding reliable messages (QoS> 0) respectively	number type input. Type checked and coerced into integer before updating v5 properties object	50%	Attempted to test by setting low value (5) but couldn't trigger the disconnect. Suspect this is something I am doing wrong in my tests
Maximum packet length. Allow the client and server to individually specify the maximum message lengths they support	number type input. Type checked and coerced into integer before updating v5 properties object	50%	Although implemented and tested, it seems a bit odd in that when requesting the msg size to be smaller, a subscription node still receives a message but the topic is empty. So it works but not as expected.
Reason codes of all confirmation packets. All response packets contain reason codes so that the caller can determine whether the requested function was successful.	Leave for future update	NA	In my trials and testing, the 'message' event never seems to contain this! Perhaps we need to handle 'packet' event instead?
Optional server function availability. Inform the client the feature list supported by the server to avoid the client using unsupported features	Upon connack, I capture the flags the broker reported.	50%	Utilise these flags and determine safe defaults. Not necessary for 1st v5 release. Can easily be utilised at a later for a later revision / when requested?
Enhanced authentication. Provide a mechanism to enable challenge/response style authentication including mutual authentication.	Leave for future update	NA	OK to leave this for a later revision / when requested?
Server Disconnect. The server is allowed to send a DISCONNECT message to indicate why the connection was closed.	The event is wired up but unsure how to handle it	50%	What to do with this event? The broker disconnects regardless so its probably for client side clean-up / visualisation / logging only?
Server reference / redirect. Allows the server to specify a standby server.	Leave for future update	NA	OK to leave this for a later revision / when requested?
Will delay. Provide the ability to delay the sending of the specified will message after the connection is interrupted	Leave for future update	NA	OK to leave this for a later revision / when requested?
Server keeps connected. Allow the server to specify the keep-alive values it wants the client to use	Leave for future update	NA	OK to leave this for a later revision / when requested?

 

Subscribe options	Implementation notes	Testing	Follow up / notes
Shared subscription. Load balancing groups (round robin)	Already implemented	100%	Round robin tested + working as expected
Subscription Identifier. Allows to specify a digital subscription identifier in the subscription packet and returns this identifier when the message is distributed	The value is parsed into integer type and passed to the v5 properties object when > 0.	75%	If a subID is present in an incoming message AND this UI option is set, this acts like a secondary filter and actually solves the multiple messages on overlapping wildcard topics. Is this a reasonable approach for NR implementation?
no local. Don’t send published messages back to same client	Checkbox flag option. Type checked and coerced into Boolean before updating v5 properties object	100%
Retain as Published. Messages forwarded using this subscription keep the RETAIN flag they were published with	Checkbox flag option. Type checked and coerced into Boolean before updating v5 properties object	100%	Works as described (though I am not certain of usefulness the feature).
Retain Handling. 0: always get retained messages, 1: get retained messages on new subscription, 2: never get retained messages	Select option. Type checked and coerced into integer 0 ~ 2 before updating v5 properties object	50%	I have verified correct value is passed to v5 properties. Seems to work but needs thought around option 1.
User Properties. Support user-defined attributes and transmission of additional custom information to expand more application scenarios	Permit JSON and flow/global. Under the hood, a helper function ensures only string pair values are sent / received	100%	Unsure of the purpose of user properties in the subscribe node! I suspect they arrive in the suback packet for use by other broker/client implementations.

 

Publish Options	Implementation notes	Testing	Follow up / notes
User Properties. Support user-defined attributes and transmission of additional custom information to expand more application scenarios	TypedInput permits JSON and msg/flow/global. Under the hood, a helper function ensures only string pair values are sent / received	Done	Should this be an editableList (or is typedInput acceptable for initial version)
Topic alias. Reduce the overhead of MQTT messages by shortening the topic name to a small integer.	Can be passed in msg.topicAlias if empty. NOTE Broker disconnects if topicAlias is used without first being set. This is per spec	Done	Should this be a typed input? (or is std input acceptable in first release)
Message expiration. Allows messages to be published with an expiration interval	Can be passed in msg.sessionExpiryInterval if empty	Done	Should this be a typed input? (or is std input acceptable in first release)
Payload format and content type. Allows to specify payload format (binary, text) and MIME-style content types when publishing messages	Can be passed in msg.contentType and msg.payloadFormatIndicator if fields are empty.	50%	The contentType is simple (passed back in the message) but unsure how to handle utf8. Is the binary/utf8 transmission handled by the client? Cant find clear details
Request / Response Pattern. Specify the MQTT request / response mode, provide the response topic and compare data attributes, and control the response message to be routed back to the request publisher	Can be passed in msg.responseTopic and and msg.correlationData if fields are empty.	Done	For this implementation, correlationData must be a buffer (as per spec). however, should we permit string/other value types in correlationData & coerce this to a buffer in the backend? The issue arises when we try to do the reverse on an incoming message. Is it acceptable to leave this as a buffer in first release?
Shared subscription. support for shared subscriptions to allow load balancing for multiple subscription consumers	Tested 3 client connections and tested the round robin effect.	Done
client identifier. If the server has assigned a client identifier, this client identifier is returned to the client.	Leave for future update	NA	Cant find clear information on this. OK to leave this for a later revision / when requested?
Subscription Identifier. An ID associated with the subscription	The value is parsed into integer type and passed to the v5 properties object when > 0 (as per spec)	Done	I cant for the life of me understand how Sub ID operates at the publish side. Oasis desc on Subscription Identifier for publish packet is unclear.

Implementation Notes

• V5 options are hidden by default and show when a user selects V5 option in the broker config node.
• Publishing or Subscribing with incorrect properties value types or incorrect combinations of values can crash node-red.
  • Added Helper functions that check bounds and coerce types added to contain this.
• Currently, MQTT Node doesn’t unsubscribe a topic until the node is deleted, in order for changed options to take effect, this probably needs changing (I have changed this see this.unsubscribe //TODO)
• For publish nodes, the v5 properties that can be sent in msg object use the exact name/case/spelling as the MQTT.JS lib for consistency. However, if all items were typedInputs, this would be moot.

Issues...

MQTT IN, MQTT OUT, MQTT Broker userProperties

• Using typedInput when set to JSON, if left empty, shows red border. How to handle this?
  • Add "none" to typed input?
  • Allow empty JSON in typedInput (via setting)?
  • Dont use typedInput
    NOTE: userProperties MUST be UTF-8 String pairs https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901116

Questions...

Should V5 properties passed in a msg be encapsulated in msg.properties?

• currently, they are passed separately in msg.userProperties, msg.topicAlias etc then manipulated into the MQTT packet under options.properties

Should V5 properties returned from a subscrption be in msg.properties?

• currently, they are passed out separately in msg.userProperties, msg.topicAlias

Should other properties (retain handling, subscription ID, content type, alias etc) also be typedInputs?

• My thoughts on this are, if (from inception) these properties care passed in via msg.topicAlias, msg.subscriptionId etc then we have to deal with that forever more if we are to maintain backwards compatibility.
• alternatively, if v5 properties are bundled into msg.properties, this is only one property that needs future handling.

Other / TODO

• Remove debug lines
• Transaltions (en are done but other langs not)
• review //TODO: comments in code

Reference Materials...

Forum discussion: https://discourse.nodered.org/t/node-red-mqtt-5-support/17847/
Oasis Standard (current): https://docs.oasis-open.org/mqtt/mqtt/v5.0/mqtt-v5.0.html
MQTT.js reference: https://github.com/mqttjs/MQTT.js#client

[coveralls]

Coverage decreased (-1.03%) to 66.6% when pulling 16088b8 on Steve-Mcl:mqtt5 into f96ce2f on node-red:dev.

[Steve-Mcl]

(@knolleary) Nick, question, should I be updating this PR with changes you guys make to the DEV branch (to keep it up to date)?

[knolleary]

@Steve-Mcl no need - unlikely to get any conflicts as we're not making changes to these files ourselves at the moment.

[natcl]

@Steve-Mcl I'm testing this this morning and was wondering if it's normal that the MQTT in node and the configuration node shows the User Properties field ? Shouldn't this be visible only on the MQTT out when publishing ?

[Steve-Mcl]

@Steve-Mcl I'm testing this this morning and was wondering if it's normal that the MQTT in node and the configuration node shows the User Properties field ? Shouldn't this be visible only on the MQTT out when publishing ?

Hi @natcl the User Properties are valid for subscriptions (MQTT IN)

3.8 SUBSCRIBE - Subscribe request --> 3.8.2.1.3 User Property

The Non-normative comment states

User Properties on the SUBSCRIBE packet can be used to send subscription related properties from the Client to the Server. The meaning of these properties is not defined by this specification

Although I have no means of testing this or fully understand the usage, I imagine its for broker developers to use.

When Nick gets around to looking at the work, he/we may decide it is not relevant or useful to include, but as far as the oasis spec goes, it is valid.

[knolleary]

Hi @Steve-Mcl

this is next on my list to look at for 1.3.... but there is rather a lot to wade through... good work 😉

Truth be told, I can't remember the semantics of a lot of this stuff as I've not done any thing with v5 for so long. So this is a fun nostalgic trip remembering 18 months of heated technical debate about what should be in v5.

At first glance, I think we need to be fairly stringent on what options we choose to expose in the UI - and how we expose them. I'm concerned about throwing everything in there at the cost of making it appear harder to use for the simple cases. It's always easier to iterate to add missing features then it is to remove unused features later on.

Here are some initial thoughts - don't take these as immediate calls for action, rather points to discuss.

1. Subscribe user props - I'm not sure they need to be exposed at this stage.
2. Subscription ID
   1. I don't think that's valid for the messages a client publishes - its used in publishes sent from the broker.
   2. I also don't think we should expose it to the end user in the Subscribe node. We can make use of it under the covers to resolve the issues with overlapping subscriptions, but thats an internal implementation detail, not an end-user feature
3. Payload Format Indicator - I think this can be removed from the UI and set via message property if the non-default value is needed.
4. Content-type - would be good to have that as a typedInput with some of the common mime types as predefined choices plus a 'none' and an 'other' option to type something else in.
5. Topic Alias - As with subscription ID, its an implementation detail between the client and broker. If we expect the user to set the alias, then they are responsible for getting the semantics right and ensuring they don't accidentally reuse an alias in two nodes. So we shouldn't expose it - and the client can make use of it under the covers if we wanted to.
6. Alias Max - no reason the user should set this. It's for the node-red client to negotiate with the server if we're going to use aliases under the covers.
7. Response topic && Correl data - Maybe hide these behind a checkbox to enable request/response messaging ? They act as a pair (although correl is optional), so would be good to reflect that in the UI
8. Receive max - given we don't provide a way for the flow to ack the message (we do it automatically) then exposing this option doesn't make much sense. We might in the future have to add some means to ack messages in the flow to provide more assurance over delivery, but that's for another day.

[Steve-Mcl]

Hi Nick. On a quick read of your points, I agree with much of what you said. What is the timeframe for getting this ready for 1.3 ?

[knolleary]

I'd like to get the beta out as soon as we can. We don't need absolutely everything in the beta, but the more that's in there, the better.

Do you think you'll get a chance to make these updates in the next week? Most of it is removing things, so shouldn't been too much work.

There's more to do on top of that with updating the help and i18n messages etc. But that's stuff I can help tidy up once the basic function is there.

[Steve-Mcl]

Do you think you'll get a chance to make these updates in the next week

Yeah, should be do-able.

Does this approach sound ok...

• merge current DEV into MQTT5 branch
• begin implementing changes, commit & push after each area of change

[knolleary]

Sounds good. Dev should merge in cleanly.

[Steve-Mcl]

Hi Nick, I'll post smaller updates for quick review by you - to see if I am heading in an acceptable direction if you don't mind?

4. Content-type - would be good to have that as a typedInput with some of the common mime types as predefined choices plus a 'none' and an 'other' option to type something else in.

Is this an acceptable solution - instead of a "none" and "other" entry, utilise a custom list and str and msg, flow, global?

ignore the double midi entry

[Steve-Mcl]

Response topic && Correl data - Maybe hide these behind a checkbox to enable request/response messaging ? They act as a pair (although correl is optional), so would be good to reflect that in the UI

This is a mock up...

The issue I have just thought of with this approach is that we will need an additional boolean for the use/no use checkbox.
(Alternatively, we would need to check that response topic is null and correl data is null & thus uncheck the check box.)

but, then, if we permit override in msg would we need to request the user also provide something like msg.reponsePattern = true;

🤔

I will leave these 2 as is until I have removed / tidied the form - perhaps it doesnt look too busy once we have thinned the other options out?

Appreciate you feedback.

[natcl]

Just wondering but perhaps response topic and correlation data could only be passed via msg ? Does it need a UI ?

[Steve-Mcl]

@natcl

response topic and correlation data could only be passed via msg ? Does it need a UI ?

Not meaning to be pedantic but as a counter argument, the same could be said for topic or QoS or retain.

The elements I see as sensible for UI definitely include

• response topic
• user properties
• content type
• expiry

Others should perhaps be either accessible via msg params or removed / hidden due to being more "under the hood" than client facing.

Thoughts?

[Steve-Mcl]

Hi @knolleary

I have been mocking and playing with MQTT OUT node. After some play time and re-reading of your comments, I have a proposal I hope is a reasonable compromise.

MQTT OUT node

Properties I believe it makes sense to be on the UI (written in order of appearance)

user properties

• typed input with the following choices...
  • none (default)
  • msg, flow, global
  • json
• NOTES
  • the underlying variable is defaulted to "userProperties". This means if the user selects type msg it has a good default msg.property set (and matches the underlying MQTT library v5 option spelling)
  • the JSON a user enters is validated / parsed to be key/string pairs only (as per oasis spec)

response topic

• typed input with the following choices...
• none (default)
  • msg, flow, global
  • str
• NOTES
  • the underlying variable is defaulted to "responseTopic". This means if the user selects type msg it has a good default msg.property set (and matches the underlying MQTT library v5 option spelling)

correlation data

• a typed input that ONLY appears when response topic is !== "none" and has the following choices...
  • none (default)
  • msg, flow, global
  • bin
• NOTES
  • the underlying variable is defaulted to "correlationData". This means if the user selects type msg it has a good default msg.property set (and matches the underlying MQTT library v5 option spelling)

content type

• a typed input that has the following choices...
  • none (default)
  • type (a drop down list of common MIME types defaulted to application/json (seems like the obvious choice)
  • msg, flow, global
  • str
• NOTES
  • the underlying variable is defaulted to "contentType". This means if the user selects type msg it has a good default msg.property set (and matches the underlying MQTT library v5 option spelling)

message expiry (secs)

• a typed input that has the following choices...
  • none (default)
  • msg, flow, global
  • num
• NOTES
  • the underlying variable is defaulted to "messageExpiryInterval". This means if the user selects type msg it has a good default msg.property set (and matches the underlying MQTT library v5 option spelling)

Rationale & logic on changing all to typedInputs...

I believe there are 3 ways this could go

• no UI at all (but permit user to send v5 options in msg.properties)

  • PRO this matches up with the v5 properties object in the underlying lib
  • CON no UI for things that probably should have UI

• some (where obvious) typed inputs

  • PRO easy config of popular items
  • CON makes no sense packing all overrides into the msg.properties object (as the user would set msg.something for the typed inputs)

• all typed inputs

  • PRO easy config of supported options

Demo based on the above...

I dont think this is too overwhelming?

Please let me know your thoughts so I can proceed with the coding.

[knolleary]

Go ahead with what makes sense and can review when you've done it.

[Steve-Mcl]

Progress...

Pretty much everything you suggested has been implemented (perhaps some took a slightly different direction due to re-think / compromise)

• MQTT IN node tidy up

  • remove userProperties
  • remove subscriptionIdentifier
    

• MQTT OUT node tidy up

  • remove topicAlias
  • remove payloadFormatIndicator
  • remove subscriptionIdentifier
    

• MQTT BROKER node tidy up

  • remove topicAliasMaximum
  • remove maximumPacketSize
  • remove receiveMaximum
  • remove userProperties
    

msg only v5 options (intended to be undocumented / experimental)...

• msg.topicAlias [integer]
  • if the user pubs with topic AND topicAlias then can then publish with only a topicAlias in subsequent pubs
  • see demo flow below.
• msg.payloadFormatIndicator [boolean]
  • Payload is UTF-8 Encoded Character Data (true) or not (false)
  • untested

demo flows...

[{"id":"f54ddee6.c1ffb","type":"inject","z":"4b3f21a3.ba434","name":"","props":[{"p":"payload"},{"p":"topic","vt":"str"},{"p":"correlationData","v":"[99,111,114,114,101,108,45,100,97,116,97]","vt":"bin"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"","payloadType":"date","x":1640,"y":820,"wires":[["ce561351.023cf"]]},{"id":"ce561351.023cf","type":"mqtt out","z":"4b3f21a3.ba434","name":"","topic":"demo/request","qos":"","retain":"","responseTopic":"demo/response","responseTopicType":"str","contentType":"text/plain","contentTypeType":"preset","userProperties":"","userPropertiesType":"none","correlationData":"correlationData","correlationDataType":"msg","messageExpiryInterval":"","messageExpiryIntervalType":"none","broker":"f03246cf.89b378","x":1800,"y":820,"wires":[]},{"id":"76b010bc.e1001","type":"mqtt in","z":"4b3f21a3.ba434","name":"","topic":"demo/request","qos":"2","datatype":"auto","broker":"","nl":false,"rap":true,"rh":0,"x":1630,"y":940,"wires":[["6ae2715b.2b5fd"]]},{"id":"5526de76.752b4","type":"mqtt out","z":"4b3f21a3.ba434","name":"","topic":"","qos":"","retain":"","responseTopic":"","responseTopicType":"none","contentType":"application/json","contentTypeType":"preset","userProperties":"","userPropertiesType":"none","correlationData":"","correlationDataType":"none","messageExpiryInterval":"","messageExpiryIntervalType":"none","broker":"","x":2130,"y":940,"wires":[]},{"id":"2eb50de0.724ce2","type":"mqtt in","z":"4b3f21a3.ba434","name":"","topic":"demo/response","qos":"2","datatype":"auto","broker":"","nl":false,"rap":true,"rh":0,"x":1980,"y":820,"wires":[["7025fee9.11ad8"]]},{"id":"7025fee9.11ad8","type":"debug","z":"4b3f21a3.ba434","name":"req res pattern","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":2160,"y":820,"wires":[]},{"id":"6ae2715b.2b5fd","type":"change","z":"4b3f21a3.ba434","name":"delete topic & add random payload","rules":[{"t":"delete","p":"topic","pt":"msg"},{"t":"set","p":"payload","pt":"msg","to":"$floor( ($random() * 10) + 1)\t","tot":"jsonata"}],"action":"","property":"","from":"","to":"","reg":false,"x":1880,"y":940,"wires":[["5526de76.752b4"]]},{"id":"c587d761.1b9078","type":"comment","z":"4b3f21a3.ba434","name":"*************** Response topic demo ************************","info":"","x":1770,"y":720,"wires":[]},{"id":"3f9b6352.5d448c","type":"comment","z":"4b3f21a3.ba434","name":"send payload to \"demo/request\" expect reply on \"demo/response\"","info":"","x":1790,"y":780,"wires":[]},{"id":"98954f96.28ccf","type":"comment","z":"4b3f21a3.ba434","name":"fake external application recieving command and responding to reply topic","info":"","x":1820,"y":900,"wires":[]},{"id":"eb961a99.9e7458","type":"inject","z":"4b3f21a3.ba434","name":"demo/aliastest (2)","props":[{"p":"payload"},{"p":"topic","vt":"str"},{"p":"topicAlias","v":"2","vt":"num"},{"p":"userProperties","v":"{\"test\":\"value\"}","vt":"json"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"demo/aliastest","payload":"","payloadType":"date","x":1670,"y":1080,"wires":[["3dc7755d.1346ba"]]},{"id":"1f0fc483.8e641b","type":"mqtt in","z":"4b3f21a3.ba434","name":"","topic":"demo/aliastest","qos":"2","datatype":"auto","broker":"","nl":false,"rap":true,"rh":0,"x":1650,"y":1160,"wires":[["9afd3163.2ec39"]]},{"id":"9afd3163.2ec39","type":"debug","z":"4b3f21a3.ba434","name":"alias teset","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":1880,"y":1160,"wires":[]},{"id":"3dc7755d.1346ba","type":"mqtt out","z":"4b3f21a3.ba434","name":"","topic":"","qos":"","retain":"","responseTopic":"","responseTopicType":"none","contentType":"","contentTypeType":"none","userProperties":"","userPropertiesType":"none","correlationData":"correlationData","correlationDataType":"msg","messageExpiryInterval":"","messageExpiryIntervalType":"none","broker":"","x":1870,"y":1100,"wires":[]},{"id":"52addaa1.e3a654","type":"inject","z":"4b3f21a3.ba434","name":"topicAlias 2","props":[{"p":"payload"},{"p":"topicAlias","v":"2","vt":"num"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payload":"","payloadType":"date","x":1650,"y":1120,"wires":[["3dc7755d.1346ba"]]},{"id":"84e1fc5b.fc612","type":"comment","z":"4b3f21a3.ba434","name":"************ topic Alias via msg.topicAlias ************ ","info":"","x":1750,"y":1040,"wires":[]},{"id":"f03246cf.89b378","type":"mqtt-broker","name":"","broker":"localhost","port":"1883","clientid":"test-v5-node-red","usetls":false,"protocolVersion":"5","keepalive":"55","cleansession":false,"birthTopic":"","birthQos":"0","birthPayload":"","closeTopic":"","closeQos":"0","closePayload":"","willTopic":"","willQos":"0","willPayload":"","sessionExpiryInterval":""}]

important note. publishing a topicAlias before registering it will crash node red. This is handled in MQTTJS #1176 (due for release soon)

[knolleary]

msg.topicAlias

I don't immediately see why we should expose this if the client is handling aliases under the covers.

To do aliasing properly, the client needs to know if the broker supports them and if so, what limits it imposes. Allowing the user to specify their own aliases with no regard for what the broker has said is allowed is a recipe for things to fail.
If the broker sets a max limit on aliases, does the client code properly manage this if the flow could set a new alias on every single publish that is sent?

[Steve-Mcl]

msg.topicAlias

I don't immediately see why we should expose this if the client is handling aliases under the covers.

I left this in (albeit an undocumented feature) as I believe it could be useful for users wanting to perform high speed / reduced traffic mqtt.

To do aliasing properly, the client needs to know if the broker supports them and if so, what limits it imposes. Allowing the user to specify their own aliases with no regard for what the broker has said is allowed is a recipe for things to fail.
If the broker sets a max limit on aliases, does the client code properly manage this if the flow could set a new alias on every single publish that is sent?

The PR does receive the max alias from the broker and stores it to ensure the user cannot use an alias when not not supported or greater than the maximum alias. At present, I cant remember exactly what I do (ignore the transmission or raise an error - but we have options if you have a preference).

Also, I am happy to disable this if it permits us to get MQTT5 merged Nick?

[knolleary]

On all the typedInputs - we don't provide flow/global options on every minor option unless there's a compelling use case for why it will be a commonly used choice.

Nor do we generally let the user change what msg properties are used. This is all about trying to reduce the number of choices a user has to make - every extra option we present is an extra things the user has to think about.

So I would like to remove the flow/global options. I can live with the msg option staying.

I left this in (albeit an undocumented feature) as I believe it could be useful for users wanting to perform high speed / reduced traffic mqtt.

My concern is that exposing an option like this, even undocumented, may mean our hands are tied in the future to make the client smarter in its auto-aliasing.

I'm happy to merge this as-is and make these changes myself. I note you also saw the require around LWT properties - they asked in Slack at the same time as posting to the forum. I'll take a look at what it'll take to add those options to the broker ui as well.

[knolleary]

Just for future reference as I'll tidy it up in merge, but need to be careful not to apply global styles like this. Needs to be more carefully scoped to this node.

[Steve-Mcl]

oops on the global style. In my head it only applied to .form-row and only for this node. After reading your commend and using a bit of brain power, I realise the CSS can affect other nodes when their form is opened.

[knolleary]

I'm not sure it makes sense to expose the 'send on new sub' option. We don't let the user dynamically subscribe, so the only time they would have a resubscribe is if they had two identically configured nodes.

I think this option should be simplified to a binary "Send retained messages" options.

[Steve-Mcl]

This was purely to follow the oasis specification Nick. It does work as described. But as you have 1st hand knowledge on the design of both node-red and mqtt, I am of course happy to go with your recommendation.

[Steve-Mcl]

On all the typedInputs - we don't provide flow/global options on every minor option unless there's a compelling use case for why it will be a commonly used choice.

Nor do we generally let the user change what msg properties are used. This is all about trying to reduce the number of choices a user has to make - every extra option we present is an extra things the user has to think about.

So I would like to remove the flow/global options. I can live with the msg option staying.

No bother.

I'm happy to merge this as-is and make these changes myself. I note you also saw the require around LWT properties - they asked in Slack at the same time as posting to the forum. I'll take a look at what it'll take to add those options to the broker ui as well.

Thats fine by me if you want to crack on but I am happy to take a look if you dont have the time?

As for topicAlias, I can remove this if that is preferred?

Just let me know before 5pm please. Ether way.

[knolleary]

I'm happy to finish this off. I appreciate all your work getting to this point - and it's just a bit of fine tuning from here.

[natcl]

It seems sad to me not to support topic aliases, there are a lot of scenarios with embedded devices/PLCs/micro controllers where this could be useful...

…

-- lecaude.com studioimaginaire.com

Le 22 févr. 2021 à 07:39, knolleary ***@***.***> a écrit :  I'm happy to finish this off. I appreciate all your work getting to this point - and it's just a bit of fine tuning from here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[knolleary]

@natcl this isn't about not supporting them. It's whether we need to expose it to the end user or not.

For every node with a hardcoded topic, we could automatically create an alias under the covers for that node without the user having to do anything.

For nodes without a hardcoded topic, we could dynamically generate them as needed - within the limits imposed by the broker.

[natcl]

What I’m wondering is if you publish to a broker where the number of topic aliases per client is low (10 seems to be the default for mosquitto) The user might want to choose which topic to alias. Perhaps a message sent every second might benefit more from an an alias than a message sent once an hour.

…

-- lecaude.com studioimaginaire.com

Le 22 févr. 2021 à 07:55, knolleary ***@***.***> a écrit :  @natcl this isn't about not supporting them. It's whether we need to expose it to the end user or not. For every node with a hardcoded topic, we could automatically create an alias under the covers for that node without the user having to do anything. For nodes without a hardcoded topic, we could dynamically generate them as needed - within the limits imposed by the broker. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[knolleary]

This has now been merged - albeit with a number of changes.

• Removed the msg/flow/global options from the typed Inputs
• Removed/simplified the extra *Type properties that had been added - they are largely redundant as the types can be inferred from the value
• Updated the broker node ui layout - added session expiry option
• Added the v5 extra options to the birth/close/will messages
• msg.topicAlias is honoured if set - see below.
• updated the help to document the message properties - although I've probably missed some.

On the question of topicAliases, I've tried to make it a little smart.

If the user sends topic="foo" topicAlias=2, we check to see if we already know about topicAlias 2 and if it equals "foo". If it is known and matches, we blank out topic so the alias is used. If it isn't known, or doesn't match, we send topic and alias as-is to update the alias on the broker side.

This means a flow that dynamically sets the topic doesn't have to worry about getting the logic right to set topic the first time and not the subsequent times.

I have not exposed topic alias in the node ui. I think my earlier ideas of having the client automatically manage the aliases is worthwhile, but also more work than I can consider right now.

[Steve-Mcl]

Hi Nick, Happy to see this. Appreciated.

I like the changes you made for typedInputs - makes things a bit neater.

In my testing I noted a couple of small points...

userProperties

if the user sends {"number1":1,"subobj":{"sub1":123}} (non key/string properties) in msg.userProperties these 3 lines permits them to get through like this...

however, if we remove them 3 lines and allow the helper function to do its thing...

the user properties are constrained to key/string values only.

Also, I see you did not use the helper function for the LWT userProperties meaning the user could add non std key/other values (and get the same odd results as above).

correlationData

correlationData typedInput now permits only string. As the oasis spec says this is a buffer. I do like that you permitted a string on the typedInput however should we not also permit bin type to make it easier/possible to enter something like [1,2,3] as the correlation data?

LWT

In setting up the lwt messages, this and this look like copy paste issues (setting willMsg by mistake)

---

Otherwise, in my testing, all working nicely.

I can raise a PR to these if you wish?

[knolleary]

however, if we remove them 3 lines and allow the helper function to do its thing...

ISTR there was a reason we needed to do those three lines... but it may be I was still getting to grips with the helper functions and how they worked. I know I misunderstood them at first.

Happy to defer to you.

correlationData

its a buffer in the sense that its just a bag of bytes - it doesn't use the two-byte length header that the protocol uses for other things it calls strings.

The question is what does the user actually want to do with it. My gut feel is this will often contain a string for sheer convenience. So I've erred on the side of caution here. On the one had we could add bin as an option - on the other, if a user needs binary data then can pass it in (putting aside the will/close/birth messages, but at this point we're deep in edge case territory).

LWT

Yeah - there was lots of copy and paste going on... not 100% surprised I missed something.

PRs for the LWT and UserProps would be welcome.

Still on the fence over correlData - will be easy enough to add in the future if the need is there.