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

187 refactor notification module #193

Open
wants to merge 17 commits into
base: development
Choose a base branch
from

Conversation

djs0109
Copy link
Collaborator

@djs0109 djs0109 commented Jul 22, 2024

closes #187

djs0109 added 2 commits July 22, 2024 13:52
BREAKING CHANGE: subscriptions model has been modified. Subscriptions are now synchronized with orion
@djs0109 djs0109 linked an issue Jul 22, 2024 that may be closed by this pull request
@djs0109
Copy link
Collaborator Author

djs0109 commented Jul 24, 2024

Functions that still cant work:

  • active button
  • attribute loading
  • Tooltip for the panel view, show the complete notification endpoint
  • re.compile('.') is showing for pattern rather than the actual content "."
  • use description as header in panel, and show id bellow, maybe also with tooltips when it is too long
  • (optional) dynamically collapsable list for showing all entities

@djs0109
Copy link
Collaborator Author

djs0109 commented Jul 26, 2024

@sbanoeon I saw that the templates of the subscriptions module are using a lot of HTMX. I personally don't have much experience with that and the other modules are not using it. What is your opinion? Should we keep it here?

@sbanoeon
Copy link
Contributor

@djs0109 I think HTMX was a good way of implementing some dynamic functionalities in the subscriptions module. I would suggest to keep it. We can further use it in new features in future too.

@djs0109
Copy link
Collaborator Author

djs0109 commented Jul 31, 2024

The entity_set gives an error of missing managementForm, which makes the validation fail.

I would rewrite a parser that gets the data of entity_id/parttern/type from the request data.

@djs0109
Copy link
Collaborator Author

djs0109 commented Sep 18, 2024

@sbanoeon could you also have a look at the issue of the attributes?

@djs0109
Copy link
Collaborator Author

djs0109 commented Oct 23, 2024

reminder: we need to pull from development

@sbanoeon
Copy link
Contributor

sbanoeon commented Dec 4, 2024

@djs0109 Could you please have a look at the changes?

@djs0109
Copy link
Collaborator Author

djs0109 commented Dec 11, 2024

@sbanoeon, you can test with the following cases:

Normal HTTP

{
    "notification": {
        "http": {
            "url": "http://localhost:1234"
        },
        "attrs": ["temperature", "humidity"],
        "onlyChangedAttrs": true
    }
}

Normal MQTT

{
    "notification": {
        "mqtt": {
            "url": "mqtt://mosquitto:1883",
            "topic": "a/test/topic"
        },
        "attrs": []
    }
}

Custom MQTT with Plain Text Payload

{
    "notification": {
        "mqttCustom": {
            "url": "mqtt://mosquitto:1883",
            "topic": "a/test/topic",
            "payload":
                "The value of the %22temperature%22 attribute %28of the device ${id}, ${type}%29 is ${temperature}. Humidity is ${humidity} and CO2 is ${co2}."
        },
        "attrs": ["temperature", "humidity", "co2"],
        "onlyChangedAttrs": false
    }
}

Custom MQTT with JSON Payload

{
    "notification": {
         "mqttCustom": {
              "url": "mqtt://mosquitto:1883",
              "topic": "a/test/topic",
              "json": {
                  "t": "${temperature}",
                  "h": "${humidity}",
                  "c": "${co2}"
              }
          },
          "attrs": ["temperature", "humidity", "co2"],
          "onlyChangedAttrs": false 
      } 
}

Custom MQTT with NGSI Payload

{
    "notification": {
         "mqttCustom": {
              "url": "mqtt://mosquitto:1883",
              "topic": "a/test/topic",
              "ngsi": {
                  "id": "prefix:${id}",
                  "type": "newType",
                  "temperature": {"value": 123, "type": "Number"},
                  "co2_new": {"value": "${co2}", "type": "Number"}
              }
          },
          "onlyChangedAttrs": false 
      } 
}

@djs0109
Copy link
Collaborator Author

djs0109 commented Jan 22, 2025

@djs0109 review reminder

@djs0109
Copy link
Collaborator Author

djs0109 commented Jan 29, 2025

@sbanoeon All test cases seem to work. But do you have an idea why payload has no default value, but json and ngsi have the initial value "null"?
grafik

)
json = forms.JSONField(
required=False,
initial=None,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is the problem above related to this variable?

@sbanoeon
Copy link
Contributor

sbanoeon commented Jan 31, 2025

@djs0109 Since payload is of type string (CharField) and the default value for it is None, the crispy form initializes it with an empty text box. As for json and ngsi, they are of type JsonField and they are initialized with null as in the screenshot. Apparently this is how the crispy form handles it.

@djs0109
Copy link
Collaborator Author

djs0109 commented Jan 31, 2025

@sbanoeon thanks! I made a little adjustment. The rest looks good to me :) we can merge and close this PR

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.

Refactor Notification module
2 participants