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.

Sign up for GitHub

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

Jump to bottom

272 mqtt v2 does not work #273

Merged
merged 10 commits into from
Jun 21, 2024
Merged

272 mqtt v2 does not work #273

merged 10 commits into from
Jun 21, 2024

Conversation

FWuellhorst
Copy link
Contributor

Closes #272
Closes #270

I also removed a redundant warning in the subscription handling, which leads to this messages:

18-Apr-2024 12:32:58 INFO: pong/NotifiedAttributesContextBroker: Posting subscription to topic '/agentlib/agentlib/myEmulatorExample/ContextBrokerSubscriptions/pong_NotifiedAttributesContextBroker' with sub'={"id":null,"description":"pong_NotifiedAttributesContextBroker","status":"active","subject":{"entities":[{"id":"ping_entity","idPattern":null,"type":"sensor","typePattern":null}],"condition":{"attrs":["ping_attribute"],"expression":null}},"notification":{"timesSent":null,"http":null,"httpCustom":null,"mqtt":{"url":"mqtt://134.130.56.157:1883","topic":"/agentlib/agentlib/myEmulatorExample/ContextBrokerSubscriptions/pong_NotifiedAttributesContextBroker/ping_entity","qos":0,"user":null,"passwd":null},"mqttCustom":null,"attrs":null,"exceptAttrs":null,"attrsFormat":"normalized","metadata":null,"onlyChangedAttrs":false},"expires":null,"throttling":null}'
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{expression: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{timesSent: (None, 4)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{http: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{httpCustom: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{qos: (0, 0)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{user: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{passwd: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{mqttCustom: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{attrs: (None, [])}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{exceptAttrs: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{metadata: (None, None)}
  warnings.warn(
d:\01_projekte\02_dzwi\filip\filip\clients\ngsi_v2\cb.py:2031: UserWarning: Different field found:{onlyChangedAttrs: (False, False)}
  warnings.warn(

However, the lines below already if both are None and handle them correctly:

            if v != ex_value:
                self.logger.debug(f"Not equal fields for key {k}: ({v}, {ex_value})")
                if not _value_is_not_none(v) and not _value_is_not_none(ex_value) or k == "timesSent":
                    continue
                return False

@FWuellhorst FWuellhorst linked an issue Apr 18, 2024 that may be closed by this pull request
mqtt v2 does not work #272
Closed
@tstorek
Copy link
Collaborator

tstorek commented Apr 23, 2024

@FWuellhorst Thanks, for the hotfix. is it correct to mix the API versions? Wouldn't it be better to completely work wirh API-Version V2? Maybe we should split this one in the hotfix and the refactoring issue.

@djs0109
Copy link
Contributor

djs0109 commented Apr 23, 2024

The hotfix is moved to #276

@djs0109
Copy link
Contributor

djs0109 commented Apr 23, 2024

I will also suggest that we should completely upgrade to VERSION2 in this PR. @FWuellhorst can you manage to implement that? Otherwise we can still find someone to take over

@FWuellhorst
Copy link
Contributor Author

It works for me, I don't have the capacity to update each client usage. We can also merge this an open a new issue.

@djs0109 djs0109 requested a review from sbanoeon May 21, 2024 13:38
@tstorek
Copy link
Collaborator

tstorek commented Jun 12, 2024

@djs0109 any news on this?

@djs0109
Copy link
Contributor

djs0109 commented Jun 12, 2024

@tstorek we are still reviewing it. If nothing important is missing, it would be merged till next week.

sbanoeon
sbanoeon requested changes Jun 13, 2024
View reviewed changes
requirements.txt Outdated
@@ -16,7 +16,7 @@ python-Levenshtein~=0.23.0
python-dateutil~=2.8.2
wget~=3.2
stringcase~=1.2.0
paho-mqtt~=1.6.1
paho-mqtt>=2.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please change it to paho-mqtt~=2.0.0 instead of paho-mqtt>=2.0.0 otherwise the code might not work if there is a major release.

setup.py Outdated
@@ -9,10 +9,11 @@

INSTALL_REQUIRES = ['aenum~=3.1.15',
'datamodel_code_generator[http]~=0.25.0',
'paho-mqtt~=1.6.1',
'paho-mqtt>=2.0.0',
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please change it to paho-mqtt~=2.0.0 instead of paho-mqtt>=2.0.0 otherwise the code might not work if there is a major release.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sbanoeon Changed !

Copy link
Contributor

Choose a reason for hiding this comment

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

@SystemsPurge you have changed in requirements.txt. Now I also changed it in setup.py

djs0109
djs0109 approved these changes Jun 21, 2024
View reviewed changes
setup.py Outdated
@@ -9,10 +9,11 @@

INSTALL_REQUIRES = ['aenum~=3.1.15',
'datamodel_code_generator[http]~=0.25.0',
'paho-mqtt~=1.6.1',
'paho-mqtt>=2.0.0',
Copy link
Contributor

Choose a reason for hiding this comment

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

@SystemsPurge you have changed in requirements.txt. Now I also changed it in setup.py

@djs0109 djs0109 merged commit d9cad8c into master Jun 21, 2024
1 check passed
@djs0109 djs0109 deleted the 272-mqtt-v2-does-not-work branch June 21, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SystemsPurge SystemsPurge SystemsPurge left review comments

@sbanoeon sbanoeon sbanoeon requested changes

@djs0109 djs0109 djs0109 approved these changes

Assignees

@SystemsPurge SystemsPurge

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mqtt v2 does not work Missing dependency in setup.py
5 participants
@FWuellhorst @tstorek @djs0109 @sbanoeon @SystemsPurge