Freq. override; Freq. definitions for "1" (mark), "0"(space); Long delay critical fix

[anklimov]

overrideFreqs (freqMark, freqSpace) function added
-1 - no override(default)
0 - no modulation (to allow direct connection output to low-freq IR input)

0 - forced frequency set-up

if freqSpace >0 - filling space period by particular frequency (default=0 - no filling)

Long delay issue fixed both, for mark and space
Ref: https://www.arduino.cc/en/Reference/delayMicroseconds

Tested

[crankyoldgit]

Hey Andrey,
I'm confused as to what you're trying to achieve here.
Can you please explain to me the use case for having an override for the modulation frequencies? This seems counter-intuitive to me. e.g. If I want to, say, send a Sony message, I'd expect sendSony() to do the correct frequency etc. I'm curious, when (and why) would I want to override that?

Also, why are you setting a modulation frequency for the "space", which is inherently an "absence" of a signal, thus really has no modulation and thus, no modulation frequency. You may have a use case, it's just not obvious to me.

Can you also expand on what the "Long delay issue" is with the NEC protocol encoding?

[crankyoldgit]

FYI, you also have a number of code issues.

e.g. The library tools & unit tests no longer compile.

$ (cd tools; make all)
g++ -DUNIT_TEST -g -Wall -Wextra -pthread -c ../src/IRutils.cpp
g++ -DUNIT_TEST -g -Wall -Wextra -pthread -c ../src/IRtimer.cpp
g++ -DUNIT_TEST -g -Wall -Wextra -pthread -c ../src/IRsend.cpp
../src/IRsend.cpp: In function ‘void xDelayMicroseconds(uint32_t)’:
../src/IRsend.cpp:133:27: error: ‘delayMicroseconds’ was not declared in this scope
     delayMicroseconds(time);
                           ^
../src/IRsend.cpp:136:24: error: ‘delay’ was not declared in this scope
     delay(time / 1000UL);  // Delay for as many whole milliseconds as we can.
                        ^
../src/IRsend.cpp:138:59: error: ‘delayMicroseconds’ was not declared in this scope
     delayMicroseconds(static_cast<uint16_t>(time % 1000UL));
                                                           ^
make: *** [IRsend.o] Error 1

i.e. delayMicroseconds() is an Arduino-only function, you need to neuter it with a #ifndef UNITTEST etc.

Plus a bunch of code "style" issues:

src/IRsend.h:31:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.h:31:  Missing spaces around =  [whitespace/operators] [4]
src/IRsend.h:33:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.h:33:  Missing spaces around =  [whitespace/operators] [4]
src/IRsend.h:34:  Missing spaces around =  [whitespace/operators] [4]
src/IRsend.h:255:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]

src/IRsend.cpp:32:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:33:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:34:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:35:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:35:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:40:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:49:  Missing spaces around =  [whitespace/operators] [4]
src/IRsend.cpp:50:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:50:  Missing spaces around =  [whitespace/operators] [4]
src/IRsend.cpp:53:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:54:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:56:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:59:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/IRsend.cpp:61:  Missing spaces around =  [whitespace/operators] [4]
src/IRsend.cpp:62:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:62:  Missing spaces around =  [whitespace/operators] [4]
src/IRsend.cpp:108:  Missing spaces around !=  [whitespace/operators] [3]
src/IRsend.cpp:109:  Missing spaces around !=  [whitespace/operators] [3]
src/IRsend.cpp:110:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:120:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:126:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:126:  Redundant blank line at the end of a code block should be deleted.  [whitespace/blank_line] [3]
src/IRsend.cpp:129:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/IRsend.cpp:129:  Extra space before ( in function call  [whitespace/parens] [4]
src/IRsend.cpp:198:  Missing space after ;  [whitespace/semicolon] [3]
src/IRsend.cpp:203:  Redundant blank line at the start of a code block should be deleted.  [whitespace/blank_line] [2]
src/IRsend.cpp:395:  Should have a space between // and comment  [whitespace/comments] [4]

[anklimov]

Hi David Frequency overriding was most generic method for me to achieve Turn-off modulation on output at all So overriding mark frequency with “0” will turn modulation off This case is useful, if output connected not to IR LED but to some input, which expect de-modulated signal (in some AV receivers for example or in-circut connection instead/after sensor) Space frequency - result of attempt to integrate ESP with some king odd device where Space is coded by some another frequency (Multiroom module). This is less useful then previous option, but defaults are working to keep normal behavior of library untouched. It just becomes more flexible. Regarding long delay issue - it looks like delayMicroseconds not working good enough for relatively long delays and this causes wrong IR header encoding for some protocols (NEC for example) And as result, initial space shorter then expected. Normally it should be about 9 ms Andrey

[anklimov]

Ok. Thanks. I will check/fix it

[crankyoldgit]

Did you try playing with the duty cycle instead? i.e. setting it to 100(%). https://github.com/markszabo/IRremoteESP8266/blob/master/src/IRsend.cpp#L81

[crankyoldgit]

The more I think about it, having a duty override makes more sense, and if it at 100(%) it doesn't stay constantly on, then we should fix that.

[anklimov]

Possibly, it should bring same result (but need to test) In any cases, DutyCycle is parameter of enableIROut() This function is invoked from almost all send<*> functions And similar changes in enableIROut required to override dutyCycle, from passed from send<*> values to 100%

[crankyoldgit]

I agree, if we want to have a duty override, then more work needs to be done, however I feel that is the better approach. i.e. no modulation == a duty cycle of 100 == "on all the time for a mark, off all the time for a space (implicit)". We already have an existing mechanism for that (setting the duty). Doing it at object creation (or via a setter function) sounds reasonable to me.

Back to the space frequency modulation. Are you saying that multi-room protocols some how encode "spaces" as pulses? (Which really means they are no longer spaces, but "marks")
Can you give me a physical example of where something requires space frequency modulation?

[crankyoldgit]

Hey Andrey,
I've tried re-implementing most of what you suggested/requested in this PR in #439

This should give you what you want. That is, a way to turn off frequency modulation(FM) completely when sending. It also has unit tests and fixes a few accidental/subtle bugs in this PR.

It does not add an overriding FM option, as you haven't convinced me (yet) that is something that's really required or beneficial.

David

[anklimov]

Thank you, David It is enough for most generic cases Freq. override I’m using in some integration/reverse engineering exercises (AV Receiver Harmann-Cordon, A-BUS protocol). Work is not done yet. Not sure, it will required finally. Andrey

…

On Mar 27, 2018, at 2:40 PM, David Conran ***@***.***> wrote: Hey Andrey, I've tried re-implementing most of what you suggested/requested in this PR in #439 <#439> This should give you what you want. That is, a way to turn off frequency modulation(FM) completely when sending. It also has unit tests and fixes a few accidental/subtle bugs in this PR. It does not add an overriding FM option, as you haven't convinced me (yet) that is something that's really required or beneficial. David — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#438 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVhOGbhXvNRORteBU2-reeAqnHNhqYvFks5tiiUbgaJpZM4S6XBR>.

[crankyoldgit]

FYI @anklimov v2.4.0 has been released with the alternative PR included.

[crankyoldgit]

I think this is obsolete now.