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

add mqtt position forwarder and refactor mqtt event forwarder to use shared code #5202

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ivanfmartinez
Copy link
Contributor

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

/*
* Device alias to be used when publishing data
*/
private static final String MQTT_ALIAS = "mqtt.alias";
Copy link
Member

Choose a reason for hiding this comment

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

What is this parameter?

Copy link
Contributor Author

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

You can see on : https://github.com/traccar/traccar/pull/5202/files/34c88d44dcec94ed13b87eae516581602a0d2362#diff-a232b34e7d9106c3ae8030135f5038e844dc0f6550c8f73f78028a396e9d3195R48

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

@ivanfmartinez
Copy link
Contributor Author

@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) {
Copy link
Member

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?

Copy link
Contributor Author

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"

Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

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.

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Member

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.

Copy link
Contributor

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...

Copy link
Member

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.

@ramonsmits
Copy link

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.

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.

None yet

5 participants