HA Discovery and DMX Dimmers

[meijer3]

Pull Request Checklist

Check if the tasks below are relevant for your PR, to mark an item as completed use [X].

• [x] Updated the PLCOpen XML export file (only relevant on code change).
• [x] Updated the CoDeSys V3 export file (only relevant on code change).
• [x] Updated or created any relevant documentation.

Proposed Changes

This adds (for now only dimmers) a auto discovery to Home Assistant DMX artnet is also added to dimmers. A simple MQTT logger is also included

Related issues

• Fixes #77,
• Closes # 77

[meijer3]

I see that prettier, made the markdown scripts, prettier.... It changes some of the formatting

[meijer3]

I see I need to fix a small thing.

[meijer3]

Dimmer was not dimmable in the first version. Now it is. In the meanwhile, I also added a meta tag. So in MQTT you can see some extra info if you want. Location/id or something. Is a bit faster than spinning up a VM/éCOCKPIT

[meijer3]

And please notice that I changed FB_OUTPUT_DIMMER_MQTT.InitMqtt to straighten sub/pub topic buildup.

[MichielVanwelsenaere]

Thanks for your work! I'll give this a thorough check but I see that the ecp is not yet updated. Could you encoperate your changes in the ecp as well?

The ecp is what is used by new adopters for their own project. It's also what is used to update the exports (Codesys V3 and PLC open). The exports can then be used by people that aleady adopted the project to update their FB to the latest version.

So you see, we need both unfortunattely. I know it's a bit cumbersom, it's a pitty that these PLC projects always are binary files.

[meijer3]

Well yes I ll update the ecp tonight. I don't change the hardware tho? Nearly impossible to check then? Do you know there is subversion for éCockpit (30 day trial...)? It shows all the changes.

[meijer3]

I created a new doc for merge/pull request. It is inside this PR

[meijer3]

Done, I updated the .ecp and docs. The discovery is only for the dimmers available for now.

[MichielVanwelsenaere]

I'm missing docs on the FB_MQTT_LOG function block

[meijer3]

I added the docs. Maybe you want to straighten it all to your own style? If you think the rest is okay there is still work to do

• Add more logging (now its only startup, down and dmx error)
• Add all discoveries
• The mqtt_logger is global, via MqttVariables, maybe you don't want that, since you dont work so much with globals.

[MichielVanwelsenaere]

• startup

I'll indeed finetune the docs a bit more in a follow up PR. Seems like I forgot to finish my review yesterday, could you have a look at the remaining open points.

Almost there :)

[MichielVanwelsenaere]

Closes #128

[meijer3]

Hmm It looks like mqtt is not working as an input sub, maybe I did something wrong here. I need to fix that. Copied something old or something

//      THIS^.OUT_Internal:=STRING_TO_BYTE(Data.PayloadString^);
        THIS^.OUT_Target:=STRING_TO_BYTE(Data.PayloadString^);
        THIS^.SoftDimToValue:=TRUE

[MichielVanwelsenaere]

Hmm It looks like mqtt is not working as an input sub, maybe I did something wrong here. I need to fix that. Copied something old or something

//        THIS^.OUT_Internal:=STRING_TO_BYTE(Data.PayloadString^);
      THIS^.OUT_Target:=STRING_TO_BYTE(Data.PayloadString^);
      THIS^.SoftDimToValue:=TRUE

perhaps consider reverting that change for another PR where you can also introduce more dimming curves? This is what's in master:

ELSIF OSCAT_BASIC.IS_CC(str:= Data.PayloadString^,cmp:='0123456789.') AND THIS^.PRIO_HIGH = FALSE THEN
        THIS^.OUT:=STRING_TO_BYTE(Data.PayloadString^);
    END_IF

[meijer3]

I copied the old code from few weeks ago, but now it hangs on this. I dont know how to debug this. Never understood where the error message was.

[meijer3]

I added discovery for switches already, I can push that tomorrow or day after To add HADiscovery and update InitMqtt for other FB (docs for future us)

1. Change InitMqtt to this base.

   
   THIS^.pMqttPublishQueue := pMqttPublishQueue;
   THIS^.InstanceNamePt := CommonTypesAndFunctions.FindLastDot(ADR(THIS^.InstanceName)) + 1;

// Publish (Out); THIS^.MqttPublishTopicSuffix := THIS^.InstanceNamePt^; THIS^.MQTTPublishTopic := CONCAT(MQTTPublishPrefix^, THIS^.MqttPublishTopicSuffix);

// Subscribe (In) THIS^.MQTTSubscribeTopicSuffix := THIS^.InstanceNamePt^; THIS^.MQTTSubscribeTopic := CONCAT(MQTTSubscribePrefix^ ,THIS^.MQTTSubscribeTopicSuffix);

// register the FB agains the collector so mqtt events can be received pMqttCallbackCollector^.put(instance:= THIS^);

InitMqttDone := TRUE;

2. Remove `InstanceNamePt: POINTER TO STRING` from VAR in InitMqtt and move it to the main FB
3. Add this at the main FB VARs

MqttPublishTopicPrefix: POINTER TO STRING; MqttPublishTopicSuffix: STRING(50); MQTTPublishTopic: STRING(50); MqttSubscribeTopicPrefix: POINTER TO STRING; MqttSubscribeTopicSuffix: STRING(50); MQTTSubscribeTopic: STRING(50);

4. Replace all topics from:
`Topic := CONCAT(MqttPublishTopicPrefix^, MqttPublishTopicSuffix),`
to 
`Topic := MQTTSubscribeTopic,`

However, if every mqtt FB has this, maybe we can extend a base mqtt FB

[meijer3]

I think now its done and know that I added libraries. Like

• SM3_Shared for SM0 (system/project details, dont know if they work for codesys)
• pro_json 15 Otherwise the application needs to be called Application You should test thoroughly, with the extra libraries and such.

I am glad that you can review my work. At work nobody checked my code for like 5 year at two clients... Thanks for your time!

[MichielVanwelsenaere]

yes that's a good idea. Yet not all MQTT function blocks needs subscriptions but that could be -perhaps- solved by making the method on the base private and call it 'baseInitMqtt' and call that one from a public method 'InitMqtt' on the extended FB. On the latter we can then only request what we need and pass empty values to the base for subscription values in some cases. (Thoughts for a latter PR).

I had another look at your code and I'm under the impression that the objectId and UniqueId is not prefixed with the projectName, is that correct? It's up to you if you want to add that in this PR or I'll do it in a follow up PR as I have multiple projects for multiple devices in my home automation setup (there's a barn with a separate device). Without the prefix I wouldn't have unique Id's in Home Assistant.

[MichielVanwelsenaere]

I think now its done and know that I added libraries. Like

* SM3_Shared for `SM0`  (system/project details, dont know if they work for codesys)

* `pro_json 15` Otherwise the application needs to be called `Application`
  You should test thoroughly, with the extra libraries and such.

I am glad that you can review my work. At work nobody checked my code for like 5 year at two clients... Thanks for your time!

I'm glad your time and effort on this. I gave it my best review now but will do some extensive testing later on a device.

[meijer3]

I continued a bit. Added a general MQTT_FB and added some more auto-discoveries. Let me know if you want to add it in this PR (not done yet, but I get the hang of it)

[meijer3]

I had another look at your code and I'm under the impression that the objectId and UniqueId is not prefixed with the projectName, is that correct? It's up to you if you want to add that in this PR or I'll do it in a follow up PR as I have multiple projects for multiple devices in my home automation setup (there's a barn with a separate device). Without the prefix I wouldn't have unique Id's in Home Assistant.

Now I do, but is it not a problem inside the rest of the code? Shouldn't we implement the device name inside the mqttprefix? Devices/PLC/House/availability

Shouldnt all URLs not be in the global MQTTVariables? The example ecp shows: PLC_GROUP_MAIN => MqttSubDimmerPrefix :STRING(100) := 'Devices/PLC/House/In/Dimmers/'; MQTT_SUBSCRIBE => Topic:= ADR('Devices/PLC/House/In/Dimmers/#'), ';

Maybe just be a bit less flexible and add something like

<prefix>/<device>/In/<entity>

Where

• prefix, global fixed string
• devices comes directly from the settings in device (e.g. 'PLC_Home'),
• entity a variable is people can name them selfs? e.g. 'Dimmers'

So you have only 2 globals, and one for every entity So that the urls are more autogenerated and fixed.

[MichielVanwelsenaere]

go ahead and push all you want, on Tuesday I can do a final review and let's focus then on merging it. I've got some stuff I need to merge as well and the binary files are troublesome for parallel branches.

Concerning your MQTT questions; YES, things are to spread out for sure. I was also planning on grouping everything in MQTTvariables.

While I understand your thoughts on the auto generated topics, I wouldn't take away this configuration possibility from the end user. I think the topic structure used now is great but for sure it will not suit everyone. No?

[meijer3]

I think I am done with coding for this PR. Done:

• Move most MQTT to FB_MQTT_BASE
• Added general HA mqtt type (generates nulls for now inside the json, but its okay for now for me)
• Moved all topics to MqttVariables
• DMX chrashed on downloads, but seems fixed. A pull-the-plug restart was the only thing that helped...
• Added two more general docs. Didnt like the copy pasting so much, but feel free to copy past if you like that more.

Still on my list in an other PR

• Improve dimming, I added curves and a blink but they are not active

[meijer3]

Didnt you had any troubles with JSON_PRO 15 vs 12? MQTT needs 12. If MQTT is lacking 12 but 15 is a main lib does work, but trhows an error... 12 => Application needs to be called Application

[meijer3]

• MqttHighRequest: & MqttLowRequest, is in Dimmer, Bistable and switch.
• ExactlyOnce it is. (copied it from some FB)
• Clean the MQTT further, altho it generates more input for device.
• You can change the dim curve already, but not via parameters. See it like a feature flag
• Ill clean up these files.

[MichielVanwelsenaere]

Didnt you had any troubles with JSON_PRO 15 vs 12? MQTT needs 12. If MQTT is lacking 12 but 15 is a main lib does work, but trhows an error... 12 => Application needs to be called Application

I had to install the PRO_JSON library you added in the 'libraries' folder, after that everything compiles fine. Unfortunnately I don't have a device ready to perfom in depth tests with it.