merbanan / rtl_433

Program to decode radio transmissions from devices on the ISM bands (and other frequencies)
GNU General Public License v2.0
6.13k stars 1.32k forks source link

Acurite 5N1 triple messages #587

Closed USAFPride closed 5 years ago

USAFPride commented 7 years ago

This was reported once in the original implementation here. Line 128 acknowledges the data is sent 3 times but there is no coding in place to handle it.

acurite_00275rm_callback has code in place to fix the output of the triple results, but acurite_txr_callback (line 528 of acurite.c) appears to be coded at different times and with a different structure. To just swap the code out, would take a bit of a rewrite and I'm afraid my C coding skills are scary at best.

zuckschwerdt commented 7 years ago

Something like bitbuffer_find_repeated_row() should be used, I guess. Perhaps a general function to compute a majority rule. on bitbuffer rows. I.e. some bitbuffer_majority_decode() similar to bitbuffer_manchester_decode().

rct commented 7 years ago

First, the 5-n-1's packet repeats aren't identical. They have an indication of which iteration it is in every message.

The 275 call back makes a bunch of poor assumptions in my opinion and should be cleaned up. There are many opportunities for legitimate data to be discarded because there weren't the required number of identical repeats. Initially the message integrity checks weren't worked out, so comparing repeats was more important, but I think now there some per message check, checksum, crc, etc.

The right place, in my opinion to suppress duplicates is in your consumer, where you can make your own application specific decisions about what constitutes a duplicate on a per device and per data element basis. For example, for temperature and humidity which change slowly, you might want to suppress duplicates using a 5 minute bucket. For wind speed, especially wind gusts/peaks, you'll likely want to capture and process the data every 18 seconds.

I use rtl_433 in different ways in different times/applications. I use it for more than just for weather monitoring. It's quite handy to have a fairly inexpensive and portable data logger when you can buy wireless probes for $10-15 USD.

zuckschwerdt commented 7 years ago

I took it that the reporter was talking about https://github.com/merbanan/rtl_433/blob/master/src/devices/acurite.c#L858 and that majority comparing/combining a few lines down. That assumption then is that a single transmission burst contains 3 repeats of the same packet.

rct commented 7 years ago

Subject line is 5N1 weather sensor and OP appears to be talking about the TXR callback which was primarily written by me when I implemented the "Tower" sensor (592TXR, which I then added the cleaned up/rewritten 5-N-1 which uses a very similar message structure. (There were a number of problems with the original 5N1 not decoding things correctly.)

USAFPride commented 7 years ago

I'm specifically talking about the 5N1 per the code (line 179) static uint16_t acurite_5n1_getSensorId(uint8_t hibyte, uint8_t lobyte){ // The sensor sends the same data three times, each of these have // an indicator of which one of the three it is.

So either it sends the same data or it doesn't. The code says one thing, and you are saying something else. My logs say the data is exact (other than the first, second, or third copy byte)

khkremer commented 7 years ago

Correct, the 5N1 - as all other Acurite sensors I've seen - transmits the data several times. Your "consumer" needs to be smart enough to filter out the duplicates. Take a second look at what @rct said earlier:

The right place, in my opinion to suppress duplicates is in your consumer, where you can make your own application specific decisions about what constitutes a duplicate on a per device and per data element basis. For example, for temperature and humidity which change slowly, you might want to suppress duplicates using a 5 minute bucket. For wind speed, especially wind gusts/peaks, you'll likely want to capture and process the data every 18 seconds.

That’s how I do it.

rct commented 7 years ago

The 3 iterations of the message aren't bit-for-bit identical because there are bits that indicate what copy number it is. If you choose to approach it as, ignore that part of the preamble and the message is actually the same, fine. That part of my reply, should have been explicitly addressed to @zuckschwerdt, in the context that you can't use the row comparison function to decide whether there are repeated rows in the same bitbuffer.

USAFPride commented 7 years ago

The "Data" is the same. I agree it is not identical bit-for-bit (as I stated), but what purpose does delivering the same "data" 3 times offer? Whether you query 5 minutes or 18 seconds or 5 hours, is having the same "data" 3 times really beneficial?

As I also pointed out, not all Acurite sensors output from RTL_433 is repeated (rtl_433 receives it 3x, but doesn't output it 3x). eg acurite_00275rm_callback.

Ultimately, different people coded different devices different ways and the output is not consistent across all Acurite devices from rtl_433. Sure, I can code it to ignore the triplicate data on the 5n1, but I don't need to do that on the 00275rm.

zuckschwerdt commented 7 years ago

Refer to https://github.com/merbanan/rtl_433/issues/567 for a similar discussion.

zuckschwerdt commented 7 years ago

as a side note: I'm currently working on some 1527-type devices (EV1527 / RT1527 / FP1527 / HS1527) and those will transmit for quite some time. A single packet is ~30ms long and thus ~37 packets fit into a single sample buffer. I think it is ok to output once for 37 identical rows every 0.5 seconds, but it would be bad to output once every 30ms, possibly yielding 300 outputs for a single "physical event" (motion detection).

USAFPride commented 7 years ago

Without knowing the inner workings of the matching receiver for the transmitting device, I agree with your comments in #567 that the companies are using this as a cheap way to guarantee the receiver gets valid information. Either the receivers ignore the data if all 3 are not the same or if 2/3 are the same they accept it. My bet is that all 3 must be the same for it to accept it as a valid result.

The code in acurite_00275rm_callback seems to take the same approach already. Can we guarantee that the device running rtl_433(or dongle) isn't the point of error if all 3 don't match? I can't imagine anyone using the results if at least 2/3 transmissions don't match and I can't believe anyone would accept the results if all 3 are different (when they should be the same).

rct commented 7 years ago

@zuckschwerdt - Agreed, remote controls and other devices that send a ton of repeats in a best effort to be heard are reasonable to suppress repeats in my opinion. Though in that case, it might be useful to add a synthesized data element that indicates the number of successful decodes in that bit buffer.

@USAFPride - Most of the Acurite devices have message integrity checks. In the case of the 5n1, the 592TXR "Tower", and the 6045M lighting sensor, each message contains both a checksum and parity check bit on every byte of the data payload. So no, I don't believe their console displays are trying to compare copies as a measure of integrity. It's simply not necessary. If you read the code you can see what types of message integrity checks are used for each device.

(Though there was a least one Acurite console that in the instruction manual stated the number of "bars" displayed for signal quality was based on the number of repeats successfully decoded. In other words, 4 bars meant the signal was received well enough that all the repeats were heard. Other have been shown to be doing the "right thing" and using an RSSI indicator off the receiver, so I'm a little skeptical about that implementation.)

Similarly, the current state of rtl_433 is that decoders don't have ready access to signal strength vs. noise floor. So in order to have some data for the state of my RF environment, I track the number of successful message decodes per interval. It definitely changes throughout the day. There are times when I'm receiving all three copies without issue. And there are other times when I'm only getting one successful decode. But the data is available.

As far as the 275 call back, it is true I didn't write that. I've stated already I believe it has some poor assumptions that will lead to intervals where no data will be decoded. In other words if signal quality is marginal enough that you don't get the required number of identical copies, you won't get a decode. For some applications you won't care if the interval is short enough.

If you don't trust the message integrity checks, then you've made the case yourself for providing ALL the data to your consumer for you make your own application specific decision about data quality checks.

If you are really concerned about data quality and/or redundancy you probably want to look at a window that is larger than the length of one bit buffer. You probably want to use data comparisons that span minutes, compared to data from tens (or hundreds) of milliseconds in one callback.

merbanan commented 7 years ago

@rct All hardware and signals have different design goals and it is hard to normalize every signal to the same scale. It all comes down to how much you can trust an event and how to trust everything equally.

All these parameters are hard to combine into one quality factor that is linear or logarithmic between devices. Someone once sent a patch that combined all the repeats and made bit corrections among the messages. In theory this should improve the decoding of the messages significantly. Maybe this could be used as a signal quality factor?

Other then that we could just send all the parameters we can about the signal and let the upper layer make the decision but I don't think that anyone could make good use of knowing the amount of repeats of the message in a signal.

Bits corrected divided by bits in the signal should give quality factor that is somewhat equal between sensors.

USAFPride commented 7 years ago

Ultimately, it should produce consistent output across all devices. IMO, rtl_433 should decide that the message is valid and only output it once. I haven't perused other devices, but I'm assuming only 1 result is displayed.

Ultimately, this is up to the project owner to decide. As a coder, I would expect consistency on all output of rtl_433 code and not have to figure out x device outputs once, y device outputs twice, and z device outputs thrice. I'll dig out my C skills and put in a pull request. Ultimately, acurite.c should have consistent variables and coding throughout. If it was, you could simply add a flag for "simplified output" (1 message) or "all raw messages" and set it from the command line.

merbanan commented 5 years ago

PR #908 rtl_433 -q -Y 1 -r rtl_433_tests/tests/acurite/01/VN1TXC_02.cu8 will filter out 2 duplicate messages.