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 output #1016

Merged
merged 1 commit into from
Apr 3, 2019
Merged

Add MQTT output #1016

merged 1 commit into from
Apr 3, 2019

Conversation

zuckschwerdt
Copy link
Collaborator

MQTT output support with options to

  • output JSON event data to single topic
  • output device/sensor info to nested topics

@merbanan
Copy link
Owner

Look at that. Feature looks complete, good job. Will look at the code more closely later.

@zuckschwerdt
Copy link
Collaborator Author

Thanks! I'll later add Homie and Home Assistant discovery. But this should already cover the basics. No threading needed, the poll loop ist fast enough. Mongoose is rather easy to work with and even reconnecting failed connections works well :) I always want to add one more feature or clean something up though…

@Mindavi
Copy link
Collaborator

Mindavi commented Mar 22, 2019

I'm wondering why one would build this feature in rtl_433 instead of using a simple script for posting to the right topic with e.g. of of the mosquitto tools. This adds code to maintain while it's not really the core of what rtl_433 tries to do.

I'm always more into the unix philosophy of 'do one thing and do it well' instead of adding features just because it's easy to put everything in one place.

How do you feel about that? What is the general goal of this project besides decoding airborne sensor signals?

@merbanan
Copy link
Owner

@Mindavi internal MQTT support has been on the roadmap for a while. The target is to create an easy to use tool that can receive rf signals and then re-transmit the messages elsewhere. But you are free to use rtl_433 the way you are using it currently.

// "events" topic
if (mqtt->events && data_model) {
char message[1024];
data_print_jsons(data, message, sizeof(message));
Copy link
Owner

Choose a reason for hiding this comment

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

This buffer size needs a check. 1024 bytes might not be enough for really large messages.


static void print_mqtt_string(data_output_t *output, char const *str, char *format)
{
data_output_mqtt_t *mqtt = (data_output_mqtt_t *)output;
Copy link
Owner

Choose a reason for hiding this comment

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

Unneeded cast.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

data_output_mqtt_t vs data_output_t ;) OOP-like

@merbanan
Copy link
Owner

@zuckschwerdt code looks clean, merge whenever. The id-generation looked quirky, add some text about that. IPv6 and encryption support can be added later as needed.

@zuckschwerdt
Copy link
Collaborator Author

In my mind rtl_433 should be the go-to tool to extract structured values from radio data packets and deliver them to consumers. Fully interactive or as robust daemon. (We might offer different programs for those two someday.)

Developing sensor decoders needs debug tools and a concise screen output. Basic logging needs text output. Time series logging and home automation needs network protocols. (Sockets -- udp, tcp, unix -- are the choice for daemons, pipes are rather for simple scripts.)

Bottom line, if it's a well established standard matching our data structures and can be integrated without 3rd-party libs then we might support it if there is an active user base.

Btw. MQTT is the single most requested feature in the last years ;) (We tried multiple concepts to implement it as elegant an unintrusive as possible.)
And a HTTP (REST, WS, JSON-RPC) API and Browser-UI for optimal integration and best first impression is the next feature to land.
Support for InfluxDB, Graphite, Collectd, Statsd are all on my list for consideration too. Basically that's all just a slightly differently layed out data string on plain UDP or TCP -- but there don't seem to many users.

ctx->opts.password = pass;
//ctx->opts.keepalive = 20;
//ctx->qos = 1;
//ctx->retain = 0; // maybe 1 after testing
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess you probably want to have both qos and retain user-configurable

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does not need to be in this pull request though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion. But advanced MQTT options will have to wait a bit tough, I'll make a note.

Choose a reason for hiding this comment

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

I too would like to see 'retain' be user configurable, with home assistant its nice to have binary sensors have a state when they start up.

Additionally, this may be a bit of a big ask, but I would love to see a device filter mechanism of some sort, for example, say you are monitoring Interlogix security sensors, it would be nice to only events sent from a whitelist of devices, ignoring your neighbors sensors.

Thanks again!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll look into retain options.
Filtering may be an add-on some day. For now you could use a Python script to filter things and reinsert into MQTT.

@zuckschwerdt
Copy link
Collaborator Author

@merbanan Thanks for the review. I'll clean up and document some as suggested. IPv6 should already work, need to debug that.

@Mindavi
Copy link
Collaborator

Mindavi commented Mar 22, 2019

Well, I actually really like the feature as I'm also pushing to an mqtt server (currently through one of the mosquitto clients). I was mostly wondering where the project is headed. Every bit of code added also needs to be maintained too so I'm not always for implementing everything in one tool. Sometimes other tools are very good at doing something, which makes me think about why one would go and merge this with another tool.

I understand it's a feature that's requested often, as I'd guess lots of people want to put the messages somewhere on a message bus.

Thanks for answering 👍

@zuckschwerdt
Copy link
Collaborator Author

As a rule I always try to abstract and generalize other code bits when adding features. Just tacking on random one-off things will end with unmaintainable code, as you say.

@zuckschwerdt
Copy link
Collaborator Author

@merbanan IPv6 not working is a bug in Mongoose. S.a. cesanta/mongoose#1012

@roger-
Copy link
Contributor

roger- commented Mar 23, 2019

Awesome, and looking forward to HA discovery support! Would be nice be able to inject a custom value_template (after filtering by id and model) to support conversion of generic sensor messages.

@zuckschwerdt
Copy link
Collaborator Author

There will be a Python bridge script to translate plain events into auto discovery topics. We can tweak that until it is solid for all use cases. That way you can help to tune the value_template easily.

@zuckschwerdt zuckschwerdt force-pushed the feat-mgmqtt branch 2 times, most recently from 5b9acfa to 189a1aa Compare March 24, 2019 16:30
@doughecka
Copy link

So I've picked up a few more Acurite sensors, this time these are coming in as Acurite-Tower. I see in the code that channel is preferred over sensor ID as a way to identify the device... I think ID would be better than channel, as it should be unique... otherwise I'll be limited to just 3 sensors (on channel A, B and C). Also, it seems to be picking up ID more than once, although that may be just due to the way Acurite's 'protocol' is written.

{"time":"2019-03-28 00:39:30","model":"Acurite-Tower","id":14699,"sensor_id":14699,"channel":"A","temperature_F":70.880005,"humidity":35,"battery_ok":1}

Not sure what the best way to address this is... some might prefer using channels as a way to distinguish sensors, as long as they're ok with being capped to 3 and risking a neighbor having the same sensor. Perhaps concatenating chan ID and sensor ID if both exist? (e.g. A14699 using the example above)

@zuckschwerdt
Copy link
Collaborator Author

sensor_id is deprecated. It will be removed. Problem with IDs is that they change on battery replacement. No ideal for a small setup where the channel is the expected identifier.
Maybe needs an option…
Or are MQTT consumers flexible enough to pick up devices from topics like prefix/+/CH2 that way we could use both.

@zuckschwerdt
Copy link
Collaborator Author

Retain added and some fixes. You need git reset --hard HEAD^ ; git pull or clone again.

@Mindavi
Copy link
Collaborator

Mindavi commented Mar 28, 2019

Re filtering, isn't that possible with the sensor selection implemented already?

@zuckschwerdt
Copy link
Collaborator Author

zuckschwerdt commented Mar 28, 2019

I read the question to be different IDs in the same protocol?

@Mindavi
Copy link
Collaborator

Mindavi commented Mar 28, 2019

Ah yeah, I see. It is indeed not per sensor type but per device

@doughecka
Copy link

@zuckschwerdt I tested the 'replacing battery changes ID' thing, and the ID stayed the same. There is a reset button on the back as well, which I pressed, and the ID still remained the same. Of course other models or manufactures of sensors probably work differently, but in this case ID does what I expect. 😃

My understanding of the channel option is to help distinguish between households... I use channel A, neighbor uses channel B, etc, so our sensors don't interfere. These sensors that use channels also appear to use stronger transmitters and transmit more frequently as well, making the channel an important function in crowded environments.

On the MQTT consumer side, I'm not sure how the systems usually expect the ID... thus my thought on just combining the two fields.

@zuckschwerdt
Copy link
Collaborator Author

The general case is a new random ID on battery replacement, and e.g. a Weather station with two outdoor sensors set to Ch1 and Ch2. Channels are not RF channels but a settable indentifier bit.
Doorbell-style devices will have a settable, fixed ID. All others use a random ID an require "pairing".

@rct
Copy link
Contributor

rct commented Mar 28, 2019

Whether the ID is persistent across resets should probably be available as device meta data for consumers. That's actually an important piece of data to have for naming and storage decisions.

(Devices with volatile IDs aren't very desirable for systems that do long term storage. It might also be a useful service to document some of the device properties so people can make better buying choices for building their own systems.)

For Acurite for the devices I've worked on:

  • sensors to be used as part of their "systems" the ID is persistent. Acurite's own bridges / web service depend on that being persistent. This includes the 5-n-1 weather station, the 592TXR "tower" sensor, the newer indoor version of the tower sensor that uses the same protocol, and the 6045 lightning sensor.

  • Sensors that are intended to be used 1-to-1 with a display tend to have a volatile ID, requiring a re-pairing upon battery replacement like the 609 (and I think the 606?). The refrigerator/freezer sensor pair has a volatile ID, but has a fixed channel number to be able to differentiate the two.

  • The channels are meant to be used to differentiate sensors by people, so there are displays where you can set the channel switches (A/B/C) for one or more sensors.

@doughecka
Copy link

@rct yes, the 606 resets it's ID on power cycle. I noticed the receiver that comes with it 'pairs' only by being placed close to the sensor for a while.... I assume it's choosing the ID to pair with based on 'strength' of signal, however that is calculated.

They only transmit an ID, no channel... so for the devices that would benefit from a channel ID, don't have one. At least in Acurite-land.

@zuckschwerdt
Copy link
Collaborator Author

You can now use the option ,usechannel= to select how to use channel information:

  • replaceid (default): use a topic path of channel, fallback to id
  • afterid: append topic path of channel after id
  • beforeid: prepend topic path of channel before id
  • no: don't use channel in topic

This should cover all use-cases for small to big setups.

@zuckschwerdt zuckschwerdt merged commit 6da880f into merbanan:master Apr 3, 2019
@zuckschwerdt zuckschwerdt deleted the feat-mgmqtt branch April 3, 2019 10:40
@pbendersky
Copy link

@zuckschwerdt quick question:
can I use usechannel=beforeid combined with events?

I'm trying to run this command:
rtl_433 -f 344975000 -M newmodel -F "mqtt://192.168.1.10,usechannel=beforeid,events=rtl/sensor" -M utc

and I was expected the event JSON to be posted to rtl/sensor/<sensor_id>. However, I see the JSON event in rtl/sensor, without the id as part of the topic. Is what I want to do possible?

Thank you!

@zuckschwerdt
Copy link
Collaborator Author

events= is the topic for raw events. The topics with nested path (including channel) are under devices=. I.e you want

rtl_433 -f 344975000 -M newmodel -M utc -F "mqtt://192.168.1.10,usechannel=beforeid,devices=rtl/sensor"

@pbendersky
Copy link

Right, but that nests everything in the topic. I was hoping to get the raw JSON with the topic identified by the device ID, so I can filter it on the subscriber.

@zuckschwerdt
Copy link
Collaborator Author

Ok, I see. That makes sense, I should add an option for that. Please open an Issue asking for this so I track it.

@pbendersky
Copy link

Thank you! Create #1043 to track it.

@doughecka
Copy link

doughecka commented Apr 8, 2019 via email

@zuckschwerdt
Copy link
Collaborator Author

@doughecka something like that would be nice, yes. Perhaps add that at #1043 so we can work out what would be the best format.

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.

8 participants