uwekaditz commented 2 years ago

Please check if the PR fulfills these requirements
[x] Tests for the changes have been added / modified (needed for for bug fixes / features)
[ ] commandref has been added / updated (needed for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
[ ] Bugfix (please link issue)
[x] Feature enhancement
[ ] Documentation update
[ ] Unittest enhancement
[ ] other
What is the current behavior? (You can also link to an open issue here, if this describes the current behavior)

Some Somfy RTS telegrams are not accepted because they seems invalid

What is the new behavior (if this is a feature change)?

Telegrams are accepted and dispatched

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

no

Other information:

CHG: mcBit2SomfyRTS() remove leading '0' in any Somfy telegram if this bit it is not expected

codecov[bot] commented 2 years ago

Codecov Report

Merging #1112 (2cb24cc) into master (e17afb9) will increase coverage by 0.09%. The diff coverage is 96.15%.

@@            Coverage Diff             @@
##           master    #1112      +/-   ##
==========================================
+ Coverage   67.53%   67.62%   +0.09%     
==========================================
  Files         133      134       +1     
  Lines        9802     9830      +28     
  Branches     1570     1570              
==========================================
+ Hits         6620     6648      +28     
  Misses       1887     1887              
  Partials     1295     1295

Flag	Coverage Δ
fhem	`57.14% <16.66%> (+0.01%)`	:arrow_up:
modules	`67.62% <96.15%> (+0.09%)`	:arrow_up:
perl	`90.46% <96.15%> (+0.29%)`	:arrow_up:
unittests	`67.62% <96.15%> (+0.09%)`	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
FHEM/lib/SD_Protocols.pm	`78.57% <83.33%> (+0.08%)`	:arrow_up:
t/SD_Protocols/02_mcBit2SomfyRTS.t	`100.00% <100.00%> (ø)`
t/FHEM/14_Hideki/00_load.t
t/FHEM/14_FLAMINGO/09_parseData.t
t/FHEM/14_SD_WS/09_parseData.t	`94.11% <0.00%> (ø)`
t/FHEM/14_SD_AS/00_load.t	`100.00% <0.00%> (ø)`
t/FHEM/10_SD_GT/00_load.t	`100.00% <0.00%> (ø)`

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

sidey79 commented 2 years ago

CHG: mcBit2SomfyRTS() remove leading '0' in any Somfy telegram if this bit it is not expected

You have not provided out standard PR template.

Can you explain the problem which this fixes in more detail?

uwekaditz commented 2 years ago

I got two Somfy RCs. Without my changes I could never get a correct decoded message. In the FHEM forum is mentioned that most of the RC have a transmit message protocol starting with bits representing the character 'A'. If there is a '0' bit in front of the 'A' it will be discarded and the message starts correctly with the 'A'. At the end of the message I added a '0' bit to keep the correct length of 80 bits. I'm not sure if this '0' is correct, but so far it works for my 2 Somfy RCs. With this change I can decode all buttons from my RCs correctly and I never got a wrong transmit telegram. May be it would be necessary to check not only for the 'A' but for '8' and 'F' as well as it is mentioned in the post.

sidey79 commented 2 years ago

I got two Somfy RCs. Without my changes I could never get a correct decoded message. In the FHEM forum

Do you have some rawdata for all this sitiations?

uwekaditz commented 2 years ago

No, each RC has different raw data and each button on the RC sends also a different message. With my code change I can detect the buttons UP, DOWN and STOP correctly from each RC.

uwekaditz commented 2 years ago

Here are the raw data from my 2 Somfy RCs. They were created in FHEM with verbose=5 for the SIGNALduino device.

Here are the device settings:

define sduinoSOMFY SIGNALduino 192.168.xxx.xxx:23
attr sduinoSOMFY addvaltrigger DMSG
attr sduinoSOMFY debug 0
attr sduinoSOMFY devStateIcon {my $devIcon = 'it_wifi@gray';;;;$devIcon='it_wifi@#F5830A' if (ReadingsVal($name,"state","disconnected") eq "opened");;;;"<div>".FW_makeImage("$devIcon","$devIcon")." </div><div>".ReadingsVal($name,"state","disconnected")." / ".ReadingsTimestamp($name,'ping','')."</td></div>"}
attr sduinoSOMFY group Devices
attr sduinoSOMFY hardware ESP8266cc1101
attr sduinoSOMFY icon icoESPEasy
attr sduinoSOMFY room SOMFY
attr sduinoSOMFY verbose 5

READINGS:

setstate sduinoSOMFY opened
setstate sduinoSOMFY 2022-08-07 18:55:33 cc1101_config Freq: 433.420 MHz, Bandwidth: 325 kHz, rAmpl: 42 dB, sens: 8 dB, DataRate: 5.60 kBaud
setstate sduinoSOMFY 2022-08-07 18:55:33 cc1101_config_ext Modulation: ASK/OOK
setstate sduinoSOMFY 2022-08-07 18:55:34 cc1101_patable C3E = 00 84 00 00 00 00 00 00 => 5_dBm
setstate sduinoSOMFY 2022-07-09 18:41:03 cmds V R t X S P C r W s x e
setstate sduinoSOMFY 2022-07-09 17:52:11 config MS=1;;MU=1;;MC=1;;Mred=1
setstate sduinoSOMFY 2022-07-09 17:52:38 freeram 41864
setstate sduinoSOMFY 2022-08-12 20:25:15 ping OK
setstate sduinoSOMFY 2022-08-07 18:55:32 state opened
setstate sduinoSOMFY 2022-07-09 20:18:24 uptime 0 00:03:51

INTERNALS:

TYPE            SIGNALduino
cc1101_available    1
eventCount      26263
sendworking     0
version         V 3.5.0 SIGNALESP cc1101 (chip CC1101) - compiled at Jul 10 2022 18:28:04
versionProtocols    1.42
versionmodul        3.5.3

Somfy_Raw_data.txt

sidey79 commented 1 year ago

@uwekaditz

There was a syntax error which i fixed.

But there is a problem within the raw data

The bitlengt and the real number of bits do not fit. Also there the function does not provide the return value as seen in your logs. How did you create thos logs?

Parse_MC, extracted data 010100000100000001000001100110100110010001100000101010101100010000000000000100010000 (bin)
lib/mcBit2SomfyRTS, bitdata: 010100000100000001000001100110100110010001100000101010101100010000000000000100010000 (81)
lib/mcBit2SomfyRTS, bitdata: 10100000100000001000001100110100110010001100000101010101100010000000000000100010, truncated to length: 80
Dispatch, YsA0808334C8C155880022, test ungleich: disabled

uwekaditz commented 1 year ago

I just added the two lines code in Sd_protokol.pm and copied just this unit into my FHEM installation. The logs were created in FHEM wirh verbose=5.

sidey79 commented 1 year ago

I had no luck to reconstruct your log output. :-(

May you can double the Code check, because in your commited version, there was an perl syntax error caused massive malfunction.

uwekaditz commented 1 year ago

I'm sorry that I created so much trouble with my PR. Here is my working sequence:

In my current FHEM installation in SD_Protocols.pm (v2.05, line 977, file is attached as SD_Protocols.pm.org.txt) I changed the following code

if ($mcbitnum == 57) {
$bitData = substr($bitData, 1, 56);
$self->_logging( qq[lib/mcBit2SomfyRTS, bitdata: $bitData, truncated to length: ]. length($bitData), 4 );
}

to


# remove leading '0' in any Somfy telegram if it is not expected
if ($mcbitnum == 57 || ($mcbitnum == 81 && substr($bitData,0,1) eq '0'))  {
# length not correct, byt leading '0' -> remove leading '0'
$bitData = substr($bitData, 1, $mcbitnum - 1);
$self->_logging( qq[lib/mcBit2SomfyRTS, bitdata: $bitData, truncated to length: ]. length($bitData), 4 );
}
elsif ($mcbitnum == 80 && ((substr($bitData, 0, 5) eq '01010') || (substr($bitData, 0, 5) eq '01000') || (substr($bitData, 0, 5) eq '01111')) {
# length correct but telegram does not start with character 'A', '8' or 'F' , remove leading '0' and add a '0' at the end, see https://forum.fhem.de/index.php/topic,72173.msg1075881.html#msg1075881
$bitData = substr($bitData, 1, $mcbitnum - 1);
$bitData = $bitData . '0';
$self->_logging( qq[lib/mcBit2SomfyRTS, bitdata: $bitData, start from Bit1: ]. length($bitData), 4 );
}


and added the comments on top of the file (file is attached as SD_Protocols.pm.txt)

2. I tested the code in FHEM and it worked
3. on Github I created a repository forked from https://github.com/RFD-FHEM/RFFHEM
4. within the repo I did the same changes for SD_Protocols.pm
5. I didn't compile because I do not have a perl environment on my PC **but created the PR directly**
6. I did not copy the SD_Protocols.pm from the repo into my FHEM installation!

> I always use the attached file in FHEM.
> I excluded SD_Protocols.pm in FHEM from update.
> The logout was created in FHEM with the above changes.

I hope this will help.
[SD_Protocols.pm.txt](https://github.com/RFD-FHEM/RFFHEM/files/9542154/SD_Protocols.pm.txt)
[SD_Protocols.pm.org.txt](https://github.com/RFD-FHEM/RFFHEM/files/9542155/SD_Protocols.pm.org.txt)

sidey79 commented 1 year ago

I'll try to check this. Thanks for support

sidey79 commented 1 year ago

I took the changed code lines you send me:

I hope this will help. SD_Protocols.pm.txt

But there is a syntax error, one bracket is missing in the elsif statement :

Did you took this file from your fhem installation?

sidey79 commented 1 year ago

@uwekaditz

I think i found the problem, was not on your side, was on my side :(

RFD-FHEM / RFFHEM

Update SD_Protocols for received Somfy telegrams (remove leading '0') #1112

Codecov Report