Adds x10_sec device for decoding X10 Security RF signals #671
Conversation
src/devices/x10_sec.c
Outdated
| "model", "", DATA_STRING, "X10 Security", | ||
| "id", "Device ID", DATA_STRING, x10_id_str, | ||
| "code", "Code", DATA_STRING, x10_code_str, | ||
| "remarks", "Remarks", DATA_STRING, remarks_str, |
There was a problem hiding this comment.
remarks should probably be named event
There was a problem hiding this comment.
Renaming to 'event'
src/devices/x10_sec.c
Outdated
| "time", "", DATA_STRING, time_str, | ||
| "model", "", DATA_STRING, "X10 Security", | ||
| "id", "Device ID", DATA_STRING, x10_id_str, | ||
| "code", "Code", DATA_STRING, x10_code_str, |
There was a problem hiding this comment.
don't output data twice. use either code or remarks.
There was a problem hiding this comment.
I think there's merit to having the hex code (code) and the human-readable event (will be 'event', 'remarks' in this version) as separate fields, e.g., for CSV or JSON output. The hex code could be in the human-readable 'event' but then you have to parse for it if you're using the output.
src/devices/x10_sec.c
Outdated
|
|
||
| static int x10_sec_callback(bitbuffer_t *bitbuffer) | ||
| { | ||
| bitrow_t *bb = bitbuffer->bb; |
src/devices/x10_sec.c
Outdated
| char x10_id_str[5]="\0"; /* string showing hex value */ | ||
| char x10_code_str[5]="\0"; /* string showing hex value */ | ||
|
|
||
| /* validate what we received ... |
There was a problem hiding this comment.
loop all rows, see new_template.c, then b = bitbuffer->bb[r];
There was a problem hiding this comment.
Changing to loop per new_template.c
src/devices/x10_sec.c
Outdated
|
|
||
| /* debug output */ | ||
| if(debug_output) | ||
| { |
There was a problem hiding this comment.
join all braces on the previous line.
src/devices/x10_sec.c
Outdated
| if(test_a==bb[1][1] && test_b==bb[1][3]) | ||
| { | ||
| /* set remarks based on code received */ | ||
| switch(bb[1][2]) |
There was a problem hiding this comment.
add a space between keywords and parens if (…
|
just add more commits to this same branch. |
src/devices/x10_sec.c
Outdated
| char x10_code_str[5] = "\0"; /* string showing hex value */ | ||
| char time_str[LOCAL_TIME_BUFLEN]; | ||
|
|
||
| for ( r=0; r<bitbuffer->num_rows; ++r) { |
There was a problem hiding this comment.
don't use space after opening parens
src/devices/x10_sec.c
Outdated
| } | ||
|
|
||
| /* get x10_id_str, x10_code_str, and time_str ready for output */ | ||
| sprintf(x10_id_str,"%02x%02x",b[0],b[4]); |
There was a problem hiding this comment.
insert space after every comma
|
looks good. The indentation appears funny in some lines. I'd prefer 4 spaces instead of tabs. |
src/devices/x10_sec.c
Outdated
| test_b = b[2] ^ 0xff; | ||
| if (test_a != b[1] || test_b != b[3]) { | ||
| continue; | ||
| } else { |
There was a problem hiding this comment.
you don't need this else, which reduces the indent for everything below. Otherwise it looks perfect now!
src/devices/x10_sec.c
Outdated
| */ | ||
| test_a = b[0] ^ 0x0f; | ||
| test_b = b[2] ^ 0xff; | ||
| if (test_a != b[1] || test_b != b[3]) continue; |
There was a problem hiding this comment.
this warning is due to integer promotion and possible sign extend on 0xff. You can get the same result without warning by:
if ((b[0] ^ 0x0f) != b[1] || (b[2] ^ b[3]) != 0xff) continue; or maybe
if ((b[0] ^ b[1]) != 0x0f || (b[2] ^ b[3]) != 0xff) continue; for consistency.
src/devices/x10_sec.c
Outdated
| #include "util.h" | ||
|
|
||
| static int x10_sec_callback(bitbuffer_t *bitbuffer) { | ||
| bitrow_t *bb = bitbuffer->bb; |
There was a problem hiding this comment.
this var is also unused. everything else looks perfect.
|
Sorry, didn't see it earlier: you need to increment |
|
Thanks for all your help, @zuckschwerdt ! |
|
anytime ;) |
|
Yes, even before the looping, I was seeing 5x CLOSE and 5x OPEN in that sample, and I see that every time. I believe the the X10 sensors always transmit events multiple times in case of reception errors. I've seen this on the W800 RF receiver too. I did not notice the 41-bits. I'm going to try recording more samples this weekend with a more appropriate antenna (although the stock RTL-SDR antenna did just fine at short range). |
|
It is common to see the same packet repeated for reliability. I wonder however why open and close are in the same run of packets? Did you quickly open and close? |
|
The sample is based on pressing the 'TEST' button on a DS10A sensor. The test button generates a CLOSE event immediately followed by an OPEN event. I can easily capture samples of the reed switch itself in operation with a delay between events. Reception should be rather strong at a distance of 20 cm! I'll upload more samples. |
|
Some quick samples with separated OPEN and CLOSE events: |
|
So a normal signal is repeated 5 times, which could be used to increase reliability. And a close/open sequence indicates the test function (and is not useful for normal operation). |
Please be kind. This addresses issue #30 and adapts code provided by the OP. Adds an x10_sec device that decodes X10 Security signals. I'm also providing a test to rtl_433_tests.