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 Quinetic Switches and Sensors #3098

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

Conversation

pazzernick
Copy link

Relates to #2267

Checksumming added: ID and Action codes are now consistent

@zuckschwerdt
Copy link
Collaborator

Thanks! Can you run ./maintainer_update.py, the protocol in rtl_433.example.conf should be commented out?

Please format the quinetic_switch_decode() doc-comment as valid markdown. First line (and first line in file) should be Quinetic Switches and Sensors. with a dot at the end. Use ## Frame Layout not divider lines, use dashes for lists, indent (only) ...PPPP SS IISCC, see other files.

Generally just check a checksum and do not print it, it's not data.

Use int id = (payload[0] << 8) | payload[1]; and then "id", "ID", DATA_FORMAT, "%04x, DATA_INT, id,

Use int action = payload[2]; and then refer to only that var.

Use b as name instead of payload, like we do in almost all decoders.

Use char *action_str and static assignment instead of copy.

The names action and action_str should something with button, see other decoders.

Don't indent inside parens like this ( strcmp( but use (strcmp(

@pazzernick
Copy link
Author

Tried to sort all of the above points as best I can.

Noticed the changes from the python script have changed instances of "SoapySDR device driver is available." Reverting that text.

@zuckschwerdt
Copy link
Collaborator

Looking good.

Parens go around the bytes, i.e. it needs to be int id = (b[0] << 8) | (b[1]); (parens around b[1] optional).

Is if (button_code < 192) { button_action = "press"; } correct? The comment says 128 is release?
I think you want if ((button_code & 0x80) == 0) { -- this tests if the top bit (128) is unset.

Checksum might just work, depending on byte order, try:

    int crc = crc16(b, 5, 0x1021, 0x1D0F);
    if (crc != 0) {
        return DECODE_FAIL_MIC;
    }

Otherwise checksum can be compared as int, i.e.

    int crc = crc16(b, 3, 0x1021, 0x1D0F);
    int chk = (b[3] << 8) | b[4];
    if (crc != chk) {
        return DECODE_FAIL_MIC;
    }

@pazzernick
Copy link
Author

Thanks for feedback.

  • rows representing "press" have button_code=[01,02,03] representing button number on switch
  • rows representing "release" have button_code=192 ( but no way to indentify button number on switch )
  • use of value 128 was in error

I'm thinking that it's best to have a field named button to help with MQTT + HASS, with the button_code int value.

Equally, it might be best to discard the "release" codes, and just emit the "press" codes which seem to identify button number.

Would it make sense to use a "channel" K/V to identify the button within switch? e.g. 1, 2, 3

@zuckschwerdt
Copy link
Collaborator

zuckschwerdt commented Nov 21, 2024

We usually use "button" as DATA_INT with 1=pressed, 0=not-pressed.
Button decoders are more likely to use unit than channel combined with state (proove) or command (newkaku) for on/off.
Generaly event would be the raw event code (what you have as button_code).
So perhaps button_code -> event and then also on press button_code -> unit, command = 1, and on release just command = 0. Or maybe button instead of command. What do you think?

Can combined pressed be transmitted, e.g. 1+2? I know that EnOcean kinetic switches support this (hold one button down and press another). We would need an output to support that then.

Someone would need to go over all our button-like decoders, capture the current practice and recommend a universal schema (with hopefully little changes needed to get all decoders in line).

@pazzernick
Copy link
Author

Thanks for your clarification and suggestions.

  • I can't see any way to identify the unique button "release" so just discarding those packets for a predicatable outcome.
  • Need to order some multi-button switches to test further.

Short of getting into "schema-like" thoughts, I've chosen to employ the following fields for Quinetic Switch:

  • id
  • channel (button integer)

One notable benefit of doing that is that rtl_433_mqtt_hass.py will add HASS entities for each button within a switch.

That means no other tweaks or adaptations, and the channel represents the known button number.

@zuckschwerdt
Copy link
Collaborator

A reasonable and minimal approach. We can refine the output later, if we find a common schema for buttons.
Btw. the support for channel (and button) as automation triggers in HASS was added with #2102 about 2 years ago. We didn't get around to review button outputs since then (#2102 (comment)) so this refinement might not happen too soon ;)

@zuckschwerdt
Copy link
Collaborator

Looks good for merge. Lets wait a week for others to get a word in. Closes #2267

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