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
add mqtt position forwarder and refactor mqtt event forwarder to use shared code #5202
base: master
Are you sure you want to change the base?
Conversation
/* | ||
* Device alias to be used when publishing data | ||
*/ | ||
private static final String MQTT_ALIAS = "mqtt.alias"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of publishing the data only on a single topic for all devices if a device have an alias defined (in device attributes) position for this device will be published on specific topic. This make easy to filter / subscribe to specific device position information
like
/traccar/position/alias
instead of
/traccar/position
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we agreed on removing configuration parameters for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have requested to remove that OPTIONS parameters.
This is the minimum that I use to integrate this with my environment, split positions by an easy to use name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not something we do for forwarding now.
… in a subclass with my specific changes
@tananaev check if this last commit are acceptable, and I will make local changes to instantiate a subclass with my needs. |
|
||
} | ||
|
||
public static Mqtt3AsyncClient createClient(final String url, final String willTopic) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is willTopic
? And why do we need to send "connected" and "disconnected" payload?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is standard a way to tell any other mqtt client that this application is connected or not.
We send "connected" when connected, and if the connection is lost the server will automatically send the "disconnected" payload to the topic. This is called last will.
An application that use this data have a way to know if this specific process is connected or not to the mqtt server.
in this case if admin configure the position forward to "/traccar/position" the code will send "connected" with retain flag to "/traccar/position/state" and when the traccar server stops and disconnect from mqtt server, the server will change this retained topic to "disconnected"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please provide a link to this standard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you can read more about this procedure : https://www.hivemq.com/blog/mqtt-essentials-part-9-last-will-and-testament/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look like a standard to me. Even the article implies that it's an optional thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think most other stuff is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Truly I don't see here anything wrong with LWT topic, it's used almost everywhere in same way as @ivanfmartinez implemented it. Through this topic all subscribers will know status of slender, if traccar suddenly dies, or disconnects from MQTT server, there's no way to know that.
@tananaev please accept this pr in full, i am also waiting for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR won't be accepted as is. It does not make sense to implement a special case just for one forwarder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it is, @tananaev , maybe you are seeing something more about it, but last will testament works like that, it set's retain flag for offline message, and keeps it updated with online message.. so that's how it's possible implement disconnected state in mqtt. Sorry if i don't understand something else...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, but I'm against including LWT in this PR.
What topics are used for position/event forwarding? It seems the current MQTT implementation isn't really following how most MQTT systems publish topics around isolated topics. For example, https://github.com/ivanfmartinez/ivanfm-traccar-mqtt seems to have topics per certain values to make is easy for subscribers to subscribe at a very granular level to minimize traffic/communication between the MQTT broker and the subscriber. |
added support to send all position data using mqtt
the Event forwarder was refactored to use same code as the new position forwarder.
this code is running fine in my personal instance and I use it to integrate with some applications that use mqtt