TrakHound / MTConnect.NET

Fully featured .NET library in C# to build MTConnect Agent, Adapter, and Client Applications. Pre-built Agents with Windows Installers. Support for Windows and Linux. Supports MTConnect Versions up to 2.3. Supports .NET Framework 4.6.1 up to .NET 8
http://www.TrakHound.com
MIT License
99 stars 39 forks source link

Qos setting not being assigned to the configuration objects #82

Open virizar opened 3 weeks ago

virizar commented 3 weeks ago

Hi @PatrickRitchie

I started testing the up to date master branch and realized that the QoS was still not getting set in the MqttRelay when sending messages

My mqtt relay config still looks like this

- mqtt-relay:
    allowUntrustedCertificates: true
    clientId: simulation
    currentInterval: 10000
    documentFormat: JSON
    port: 8883
    qos: 1
    sampleInterval: 500
    server: 192.168.161.102
    tls:
      omitCAValidation: true
      pem:
        certificateAuthority: /home/victor/venice_data/state/mt_connect/gateway_certificates/gateway-SAF-Test-ID/GroupCACertificate.pem
        certificatePath: /home/victor/venice_data/state/mt_connect/gateway_certificates/gateway-SAF-Test-ID/certificate.pem
        privateKeyPath: /home/victor/venice_data/state/mt_connect/gateway_certificates/gateway-SAF-Test-ID/privateKey.pem
      verifyClientCertificate: false
    topicPrefix: MTConnect
    topicStructure: Entity

I started printing the resulting QoS value when we parse the configuration and create the _configuration object, which was still returning 0

https://github.com/TrakHound/MTConnect.NET/blob/816844d29cdaebb59892e786fc5d6811377f2b61/agent/Modules/MTConnect.NET-AgentModule-MqttRelay/Module.cs#L39-L52

Looking into the GetConfiguration method there seems to be a line that ensures the deserializer uses pascal casing for the keys CamelCaseNamingConvention.Instance

https://github.com/TrakHound/MTConnect.NET/blob/816844d29cdaebb59892e786fc5d6811377f2b61/libraries/MTConnect.NET-Common/Configurations/AdapterApplicationConfiguration.cs#L490-L512

Here is the reference of what that naming convention implies on the library that is used to serialize and deserialize yaml https://github.com/aaubry/YamlDotNet/blob/7923dd8e600f7fea7710f3b45f3fadcfa1aa589c/YamlDotNet/Serialization/NamingConventions/CamelCaseNamingConvention.cs#L25-L52

This might cause some interesting situation since the pascal case representation of qos should be Qos, and the Configuration objects have a property called QoS which does not match

This is similar to how the tls or pem fields are set on the configurations of the MqttClients, where the corresponding fields on the configuration objects are Tls and Pem

I have made a PR where I replaced all instances of QoS with Qos in the repo and tested and that seems to do the trick

PatrickRitchie commented 2 weeks ago

Thanks! Yes I saw an issue with this as well and had to use the "qoS" in yaml for it to deserialize. I like your solution better as that makes it less prone to error.