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: rolling code transmitter decoder #1339

Closed
wants to merge 16 commits into from
Closed

ADD: rolling code transmitter decoder #1339

wants to merge 16 commits into from

Conversation

dtiller
Copy link
Contributor

@dtiller dtiller commented Mar 23, 2020

This decoder handles common rolling code transmissions. It extracts the corrected 'fixed' and 'rolling' portions of the transmission as well as the id bits and the button pressed.

// This holds previous start width 1 fixed and rolling codes for
// counter decoding once we have a start width 3 packet.
static uint8_t prev_1_a_corrected[RAW_SIZE] = {NOT_SET};
static uint8_t prev_1_r_raw[RAW_SIZE] = {NOT_SET};
Copy link
Owner

Choose a reason for hiding this comment

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

Decoders usually doesn't hold any state. And I don't understand the bit stream enough to say if it really is needed. Can you add some signal samples to the tests repository?

Copy link
Contributor Author

@dtiller dtiller Mar 23, 2020

Choose a reason for hiding this comment

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

To garner any useful data from the transmission it is unfortunately necessary to retain a small amount of state. I'd hoped that all of the data bursts could be captured in a single bitbuffer with several rows, but the delay between bursts runs afoul of the limits set forth in pulse_decode.c. Christian and I discussed this briefly in email - here's why, without changes in pulse_decode and other files, that state is needed:

  1. AFAIK RTL_433 ... can't handle trinary. That means I have to use the OOK_PCM_RZ format with the short and long widths set to 500 uS (the smallest common value). This is not a big deal, except for how to install? #2 and Different temp sensor: LaCrosse / TFA IT+ on 868Mhz #3.
  2. When I attempt to grab the data, the 60 mS gap between the packets triggers the End-Of-Packet processing on lines 349-ish in pulse_decode.c. It seems that I cannot ever get the packets either on one row or on their own row!
  3. Even if we fix how to install? #2, OOK_PCM_RZ code doesn't support the gap_limit, so I would get all of the values on a single row.

Note that I did provide for debugging output - if you use -vvvv, you get output that has the raw values in each of the packets received.

I will gladly provide sample data files once my new transmitters arrive tomorrow. I'm not inclined to post files with my actual data in them.

Below are examples of the trinary nature of the data and an example of the 60 mS gap between the '3' start-bit-width packet and the '1' start-bit-width packet.

RCR Gap Edited

RCR Data

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, lets call for help. @zuckschwerdt I looked at the pulse_demod_pcm() code and it seemed feasible to add gap_limit support. Do you concur?

Somewhere around this line:
periods = MIN(periods, max_zeros);
there could be some logic to add rows.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't even realize that the PCM slicer is the problem here. Sure, I wanted to add that anyway.

Copy link
Owner

Choose a reason for hiding this comment

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

@zuckschwerdt in pulse_detect:371 this condition seems strange:

                if (((s->pulse_length > (PD_MAX_GAP_RATIO * s->max_pulse))    // gap/pulse ratio exceeded
                        && (s->pulse_length > (PD_MIN_GAP_MS * samples_per_ms)))    // Minimum gap exceeded
                        || (s->pulse_length > (PD_MAX_GAP_MS * samples_per_ms))) {    // maximum gap exceeded

I think this also will hinder the merging of signals with long gaps. And raising the reset limit to >60ms will reduce the time resolution with regards to different signals. I am not sure what it is now but I think it is like 20ms.

It would be nice to know these metrics. For example FSK devices doesn't have this property but are still affected by it.

As always I'm not totally sure what to do :) it is becoming difficult to decode all signals.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's: below 10ms will never EOP above 100ms will always EOP, between that at longest pulse x10 will EOP. That way we currently can't have gaps longer than "longest pulse x 10" (if that's above 10ms). That's for the AM envelope, thus FM isn't limited by this?

.modulation = OOK_PULSE_PCM_RZ,
.short_width = 500, // trits are multiples of 500 uS in size
.long_width = 500, // trits are multiples of 500 uS in size
.reset_limit = 2000, // this is short enough so we only get 1 row
Copy link
Owner

Choose a reason for hiding this comment

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

  1. If this limit is high enough. Both a 1 and a 3 packet might be present in the message. Then a state should not be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dtiller
Copy link
Contributor Author

dtiller commented Mar 23, 2020 via email

@dtiller
Copy link
Contributor Author

dtiller commented Mar 23, 2020 via email

@merbanan
Copy link
Owner

@dtiller there seems to be 2 signals in the files. Is one device sending something and another answering?

@dtiller
Copy link
Contributor Author

dtiller commented Mar 24, 2020 via email

@dtiller
Copy link
Contributor Author

dtiller commented Mar 24, 2020 via email

@zuckschwerdt
Copy link
Collaborator

I.e. we'd need a ratio of at least x40. I guess we can somewhat safely raise that x10 ratio if we add gap limit support to the PCM slicer. I will test that in the next days.

@evilpete
Copy link
Contributor

evilpete commented Jun 12, 2021

this sounds a lot like the Security+ rolling code protocol ( secplus_v2.c & secplus_v1.c, pull req #1483 & #1480 )

that was based on @argilo 's secplus

same spit packets delay, both trinary data with the same 1.5ms / 0.5ms encoding timing

@drws
Copy link

drws commented Aug 9, 2021

Please split Security+ discussion into a separate issue, this PR is for general rolling code additions.

@dtiller
Copy link
Contributor Author

dtiller commented Jan 21, 2022

Closing pull request. This decoder hasn't been accepted for many months and seems to be similar to Security+. Apparently the author of Security+ was able to get retained state approved in their decoder where I could not.

@dtiller dtiller closed this Jan 21, 2022
@merbanan
Copy link
Owner

I.e. we'd need a ratio of at least x40. I guess we can somewhat safely raise that x10 ratio if we add gap limit support to the PCM slicer. I will test that in the next days.

@zuckschwerdt did this get implemented? Or should we just merge as is? I think we will loose time resolution if we increase the reset limit. And while I really don't like keeping state I don't see anyway to do without it.

@merbanan
Copy link
Owner

@dtiller the decoder is neither rejected or accepted, it is just stuck in limbo until I or someone else gets time to look at it. I was planning a PR/issues marathon but real life got in the way.

@zuckschwerdt
Copy link
Collaborator

I'll look into this again shortly. Sorry this dropped off my radar. I plan to rework state support with other things like per-decoder verbosity and logging and such. But yeah, if it works, merge and apply cleanup when time allows.

@dtiller
Copy link
Contributor Author

dtiller commented Jan 21, 2022 via email

@zuckschwerdt
Copy link
Collaborator

That would be great. Always happy to see decoders get more mature and support quirks the original author had no hardware to find.
We do have one decoder that is present in triplicate (S3318P, esperanza_ews, kedsum). But it's old and people might be depending on one or the other... :/

@zuckschwerdt zuckschwerdt self-assigned this Oct 4, 2023
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.

5 participants