Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qos setting not being assigned to the configuration objects #82

Open
virizar opened this issue Oct 16, 2024 · 1 comment
Open

Qos setting not being assigned to the configuration objects #82

virizar opened this issue Oct 16, 2024 · 1 comment

Comments

@virizar
Copy link
Contributor

virizar commented Oct 16, 2024

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

public Module(IMTConnectAgentBroker mtconnectAgent, object configuration) : base(mtconnectAgent)
{
Id = ModuleId;
_configuration = AgentApplicationConfiguration.GetConfiguration<MqttRelayModuleConfiguration>(configuration);
switch (_configuration.TopicStructure)
{
case MqttTopicStructure.Document:
_documentServer = new MTConnectMqttDocumentServer(mtconnectAgent, _configuration);
_documentServer.ProbeReceived += ProbeReceived;
_documentServer.CurrentReceived += CurrentReceived;

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

public static TConfiguration GetConfiguration<TConfiguration>(object controllerConfiguration)
{
if (controllerConfiguration != null)
{
try
{
var serializerBuilder = new SerializerBuilder();
serializerBuilder.WithNamingConvention(CamelCaseNamingConvention.Instance);
var serializer = serializerBuilder.Build();
var yaml = serializer.Serialize(controllerConfiguration);
if (yaml != null)
{
if (yaml.StartsWith("-")) yaml = " " + yaml.TrimStart('-');
var deserializerBuilder = new DeserializerBuilder();
deserializerBuilder.WithNamingConvention(CamelCaseNamingConvention.Instance);
deserializerBuilder.IgnoreUnmatchedProperties();
var deserializer = deserializerBuilder.Build();
return deserializer.Deserialize<TConfiguration>(yaml);
}

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
Copy link
Contributor

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.

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

No branches or pull requests

2 participants