xdrv_17_rcswitch.ino pulselen overwritten with rfsend command

[PepeTheFroggie]

rcswitch:
Pulselen is a part of the protocol and should not be overwritten if not specified in the rfsend command.
Overwriting it will result in the wrong (protocol1 default) pulselen for everyone not aware of the issue.

The lines 198 and 199:
if (!pulse) { pulse = 350; } // Default pulse length for protocol 1
mySwitch.setPulseLength(pulse);
should be replaced with:
if (pulse != 350) { mySwitch.setPulseLength(pulse); }

Pulse is initialized to 350 and guess what effect the if (!pulse) then has? Yes, none. If you want to use (!pulse) you cant init the value.
Same with protocol, repeat, bits, if you init them you cant ask if (!repeat) { repeat = 10; }

The whole section should look like:

[barbudor]

As per the known lyrics: "I don't subscribe to your point of view"

Although pulse is initialized to 350 at the beginning of the function, then it is read from the command line following one of 2 format : JSON or comma-separated.
The check you are pointing to to test against an invalid zero value before passing pulse to the library.

The simplified whole sequence is:

    int pulse = 350;
    if (root) {    // RFsend {"data":0x501014,"bits":24,"protocol":1,"repeat":10,"pulse":350}
      pulse = root.getInt(PSTR(D_JSON_RF_PULSE), pulse);
    } else {       //  RFsend data, bits, protocol, repeat, pulse
      pulse = atoi(str);
    }
    if (!pulse) { pulse = 350; }      // Default pulse length for protocol 1
    mySwitch.setPulseLength(pulse);

The 1st case get pulse form a JSON. Providing the current pulse value (350) insure that pulse always gets a good value even if not present in the JSON.
In the 2nd case, pulse = atoi(str); reads from the command line. Behavior of atoi() is to return 0 if the string is not a valid number.
Then, the one before last 2 lines properly identify 0 as an wrong value and set back pulse to the default 350.
And finally the pulse value is passed to the library.

With your code, pulse would be set to the library if it is different than 350.
Especially it would allow setting an illegal value of 0.
This sounds totally unsafe.

May be you can elaborate on the use-case where the current code makes a problem ?

[barbudor]

Correct, my mistake
So the attached is working ?
I'll make the PR on your name

[PepeTheFroggie]

TY.

Sorry, im quite old and i struggle with PR and git in general.

[barbudor]

#10866

[PepeTheFroggie]

Still got the (!pulse) wrong. Must be (pulse)
not: if (!pulse) { mySwitch.setPulseLength(pulse); } //(pulse == 0)
but: if (pulse) { mySwitch.setPulseLength(pulse); } //(pulse != 0)

[barbudor]

Correct
New PR : #10876