Skip to content

Conversation

@hardillb
Copy link
Contributor

@hardillb hardillb commented Dec 2, 2025

fixes FlowFuse/flowfuse#6072

Description

Adds support for creating LWT/Birth/Death messages for the FF-MQTT nodes

Related Issue(s)

FlowFuse/flowfuse#6072

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

@hardillb hardillb self-assigned this Dec 2, 2025
@hardillb hardillb requested a review from Steve-Mcl December 5, 2025 14:23
@hardillb hardillb marked this pull request as ready for review December 5, 2025 14:23
@hardillb hardillb marked this pull request as draft December 5, 2025 14:26
@hardillb hardillb marked this pull request as ready for review December 5, 2025 14:48
@hardillb
Copy link
Contributor Author

hardillb commented Dec 5, 2025

@Steve-Mcl I may have "hacked" the relinking logic (that can probably be removed now there is a real conf node.)

Copy link
Collaborator

@Steve-Mcl Steve-Mcl left a comment

Choose a reason for hiding this comment

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

quick review before pull & test shows auto linking (to platform) appears to be impossible in this PR?

Co-authored-by: Stephen McLaughlin <[email protected]>
@hardillb hardillb requested a review from Steve-Mcl December 15, 2025 13:33
@Steve-Mcl
Copy link
Collaborator

Ben, I see you have re-requested a review here.
I think you missed my review comment about the automatic link up will no longer work.

I did a local pull & try out to double check my suspicions and I can confirm the auto link up is broken.

Steps:

  1. New hosted instance
  2. Drop ff-mqtt node
  3. Deploy

Note:

  • forge endpoint /api/v1/teams/:teamId/broker/client/:username/link is never called
  • no entry in clients table
  • node sits there "connecting"
image

@hardillb
Copy link
Contributor Author

@Steve-Mcl I think I've fixed the linking now

@Steve-Mcl
Copy link
Collaborator

Steve-Mcl commented Dec 29, 2025

@Steve-Mcl I think I've fixed the linking now

Yes, link up working now 👍

However, I dont think the multiple config node containment works as expected. I imported a flow from another instance and I got 2 broker nodes:

chrome_APnZpkLaFk

Also, we really should do something with the config editor button:
image

I propose we just use the built in standard/familiar node-red config tools but remove the add button e.g. in oneditprepare, we do something like $('#node-input-btn-broker-add').hide() e.g:
chrome_I9ODL1HulH

@Steve-Mcl
Copy link
Collaborator

I propose we just use the built in standard/familiar node-red config tools but remove the add button e.g. in oneditprepare, we do something like $('#node-input-btn-broker-add').hide() e.g:
chrome_I9ODL1HulH

If this is problemaric (timing wise) then lets at least add the utility class and a label (and move it into the 2nd row) e.g.:

image

This is what would be rendered:

    <div class="form-row">
        <label><i class="fa fa-cog"></i> Config</label>
        <button id="node-input-lwt-placeholder" class="red-ui-button">LWT/Birth/Death Messages</button>
    </div>

Key parts to the above:

  1. add class="red-ui-button" to the button
  2. add a label element to have the node-red css line things up
  3. add an fa icon (cog?) to the label

@hardillb
Copy link
Contributor Author

I've gone with the second option as we don't want the drop down at all as there should only be one instance of the config node.

The import bit will need some work, but I assume this is a problem the Dashboard v2 has solved?

@Steve-Mcl
Copy link
Collaborator

The import bit will need some work, but I assume this is a problem the Dashboard v2 has solved?

You know what they say about assume ;)

Nope. at least not last time i checked. Instead, dashboard tells user (in the /dashboard) there are multiple bases. We don't have that luxury (also its clunky)

@Steve-Mcl
Copy link
Collaborator

Steve-Mcl commented Jan 5, 2026

The import bit will need some work, but I assume this is a problem the Dashboard v2 has solved?

You know what they say about assume ;)

Nope. at least not last time i checked. Instead, dashboard tells user (in the /dashboard) there are multiple bases.

Ben, I revisited this. The duplication on import issue not handled in dashboard is when 2 pages have the same endpoint URL but it seems the import of a ui-base is handled - see the below linked areas of code - in particular the use of RED.events.on 'nodes:add',...)

https://github.com/FlowFuse/node-red-dashboard/blob/fd95cf0448a0fbc5172e357e2bff2884b07178fc/nodes/config/ui_base.html#L693-L727

https://github.com/FlowFuse/node-red-dashboard/blob/fd95cf0448a0fbc5172e357e2bff2884b07178fc/nodes/config/ui_base.html#L1183-L1199

@hardillb
Copy link
Contributor Author

hardillb commented Jan 6, 2026

@Steve-Mcl I think I've got the deduplicate config node code working (after the pointer from ui-base)

@Steve-Mcl
Copy link
Collaborator

@Steve-Mcl I think I've got the deduplicate config node code working (after the pointer from ui-base)

I'll take a look later. I appreciate you are pretty much AFK now so will do what is necessary to get it approved/merged.

@Steve-Mcl
Copy link
Collaborator

bah - even with the debounce instanced it still lets additional configs in...

image

Working on it now Ben!

We cannot use this as it fires even just cursoring past it in the quick-add list.
This then causes config nodes to be built and added to flow even though user never added a node
- build config here instead of onadd handler
- ensure history is correct
- ensure UI is updated (updateConfigNodeUsers, config.refresh)
@Steve-Mcl
Copy link
Collaborator

Steve-Mcl commented Jan 6, 2026

@hardillb I have pushed a few changes - noteably:

  • fixed the debounce fn
  • removed the nodes onadd handlers - these are firing even just cursoring through the quick-add list (and were causing config node to be added when it shouldnt)
    • note - that means i had to move the config node creation to the singletonLWTConfig function - which tbf deduplicates it and makes history tracking easier.
  • added history for undo (to remove programmatically added config node)
  • ensure config node users are updated and config UI side bar is refreshed

Tomorrow i will further check UI and test operation parts - but so far so good.

@Steve-Mcl
Copy link
Collaborator

Steve-Mcl commented Jan 7, 2026

Testing:

Getting issue with updating status when publishing to a topic not permitted:

image

This only happens if you publish with QOS 1 or 2 first (to generate a "prohibited" status) then publish on QOS 0.

Investigating

UPDATE: Resolved in 9ef343d

@Steve-Mcl
Copy link
Collaborator

Steve-Mcl commented Jan 7, 2026

@Steve-Mcl
Copy link
Collaborator

Steve-Mcl commented Jan 7, 2026

Testing:

Issue with cycling connect/disconnect when LWT is adjusted and flow deployed

chrome_NIFDbsh6Pw

UPDATE: additional info

Looking at NR logs we see:

Broker mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883 disconnected client: 142 
07/01/2026 10:18:39  [info]    Connected to broker: mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883
07/01/2026 10:18:44  [info]    Broker mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883 disconnected client: 142 
07/01/2026 10:18:44  [info]    Connected to broker: mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883
07/01/2026 10:18:49  [info]    Broker mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883 disconnected client: 142 
07/01/2026 10:18:49  [info]    Connected to broker: mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883
07/01/2026 10:18:55  [info]    Broker mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883 disconnected client: 142 
07/01/2026 10:18:55  [info]    Connected to broker: mq:hosted:d0bg4xgpR5:2ac10a76-d70b-4804-9682-a86b99446b71@mqtt://172.17.202.183:1883

The number 142 is the v5 rich error code which essentially means "session take over" ref - this loosly means another connection with the same Client ID was established, kicking the older session off to allow the new one - so I suspect there is a deploy cleanup issue leaving old broker node in memory when the LWT config is edited.


UPDATE: Resolved

Fixed in a078aec by adding the on-close handler

@Steve-Mcl
Copy link
Collaborator

Steve-Mcl commented Jan 7, 2026

Testing:

birthMsg, closeMsg and willMsg options are not persisted nor applied to the MQTT messages

Investigating

UPDATE: Resolved

Settings were not being saved and therefore not applied to messages.

This is reloved in 5322132

Test results:

image

@Steve-Mcl
Copy link
Collaborator

@hardillb I have pushed all fixes and im confident this is ready now - however I should now probably not be the person to review and merge! I'll ping on eng channel see if it someone can pick up and review.

@Steve-Mcl Steve-Mcl requested a review from knolleary January 7, 2026 12:06
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.

Allow Last Will & Testemnt and Birth Messages for nr-mqtt-nodes

3 participants