8000 Add support for CED7000 timer by ppapadeas · Pull Request #2319 · merbanan/rtl_433 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support for CED7000 timer #2319

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

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Conversation

ppapadeas
Copy link
Contributor

@ppapadeas ppapadeas force-pushed the add-ced7000 branch 2 times, most recently from 5b9ba75 to 47295e1 Compare January 16, 2023 09:53
@zuckschwerdt
Copy link
Collaborator

Looks good, notes:

  • you should loop all rows and find a / the long one, not hardcode 1
  • you should search the sync word, not assume it's at a fixed pos
  • use one of reverse8(), reflect_bytes(), reflect4(), reflect_nibbles()
  • you need to extract values as numbers not strings

E.g.

    uint8_t const sync_pattern[3] = {0xaa, 0x4d, 0x5e};

    // Find row
    int row = bitbuffer_find_repeated_row(bitbuffer, 1, 6*16+3*8);
    if (row < 0)
        return DECODE_ABORT_EARLY;

    // search for 24 bit sync pattern
    bitpos = bitbuffer_search(bitbuffer, row, bitpos, sync_pattern, 24) + 24;

    if ((bitpos >= bitbuffer->bits_per_row[row])
        return DECODE_ABORT_EARLY;

    bitbuffer_invert(bitbuffer);

    // Check and decode the Manchester bits
    ret = bitbuffer_manchester_decode(bitbuffer, row, bitpos, &decoded, NUM_BITS_DATA);

    ....

    int rfid = (b[1] & 0xF) * 1000 + (b[1] >> 4) * 100 + (b[0] & 0xF) * 10 + (b[0] >> 4);
-->

@ppapadeas
Copy link
Contributor Author

Thanks for the review @zuckschwerdt ! Should I amend the commit or push another one on top, based on your suggestions?

@zuckschwerdt
Copy link
Collaborator

As you like, adding commits is likely easier. We always squash on merge anyway.

@ppapadeas
Copy link
Contributor Author

@zuckschwerdt Amended following your 4 comments. Thanks for the pointers!

@ppapadeas
Copy link
Contributor Author

@zuckschwerdt let me know when adding this is next-in-line so I can rebase properly (to make your life easier ;) )

@zuckschwerdt
Copy link
Collaborator

Rebase and correct the two line with sticky braces (if …){).
Good for merge then.

@ppapadeas
Copy link
Contributor Author

@zuckschwerdt done and done!

@zuckschwerdt
Copy link
Collaborator

Perfect, thanks!
I just noticed: "rfid" is that an ID? It should be named just "id" then. All decoders have "model", "id", …

@ppapadeas
Copy link
Contributor Author
ppapadeas commented Jan 22, 2023

"rfid" is that an ID?

Not really. It is a user-set parameter from the device menu to be able to "sync" the device with accessories and use multiple ones on the field. It is named RFID in the manual and the menu.

@zuckschwerdt
Copy link
Collaborator

Ok, so more like a "channel" settings on temperature sensors.
It would be nice if this was a random number by default, but I guess it's simply "0000"?

We really always want an "id" key to be present, this is the best we get. I kind of answers the "which (local) device is this" question. Can you document that RFID is used as ID and change the key then?

Signed-off-by: Papadeas Pierros <pierros@papadeas.gr>
@ppapadeas
Copy link
Contributor Author

@zuckschwerdt gotcha. I changed it to ID now. Thanks!

@zuckschwerdt zuckschwerdt merged commit 43a0d87 into merbanan:master Jan 23, 2023
obones pushed a commit to obones/rtl_433 that referenced this pull request Jan 30, 2023
Signed-off-by: Papadeas Pierros <pierros@papadeas.gr>
andrewjw pushed a commit to andrewjw/rtl_433 that referenced this pull request Sep 29, 2023
Signed-off-by: Papadeas Pierros <pierros@papadeas.gr>
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.

2 participants
0