Add MWM support by bwarden · Pull Request #557 · crankyoldgit/IRremoteESP8266

bwarden · 2018-10-17T06:46:56Z

Disney's Made With Magic (also known as Glow With The Show) accessories use IR to communicate, both receiving commands within areas of the park, and to synchronize with each other.

Signaling is basically 2400bps serial modulated onto IR, with mark=0 and space=1, 1 start bit (mark), 8-bits LSB-first data, 1 stop bit (space), with mark being full 38 kHz modulation and space being completely off. I adapted the Lasertag decoder, since its manchester encoding was closest to the full-bit signaling used here.

I have not tested this on an ESP8266. I tested receive using a hardware IR receiver (SiliconDust HDHomeRun) that transmits timing data over the network, received and translated to human-readable pulse/space timings via LIRC's mode2 command, piped into the mode2_decode tool that I've also adapted from gc_decode. I have not tested transmit, but converted real decoded commands into theoretical timings by hand for the unit tests.

Since the messages have variable lengths, and the encoding seems to support up to 18 bytes, I've re-used the state field in decode_results, but added some logic to use only 18 bytes for the field if no AC decoders are compiled in.

I changed getRClevel's use of match() from 3 fixed levels to accepting a variable number -- the worst case in this format is 9 consecutive marks or spaces.

I removed protected from IRsendTest's mark and space methods to support directly calling them in the mode2_decode tool, allowing me to directly inject multi-bit durations.

Adding support for Disney's Made With Magic IR protocol, formerly known as Glow With the Show.

Added command mode2_decode needs the mark and space methods from IRsend_test.h to inject events based on received timing data.

MWM devices typically wait 30ms between messages, so increase footer to that length. We're re-using the state field for MWM messages, but since payload field is 4-bits with a minimum of 3 bytes, we only need 18 bytes. Reworked definition of state field in union to use only what we really need for what's defined.

crankyoldgit · 2018-10-17T14:21:32Z

First off, wow. Congratulations on doing this. Adding a protocol and unit tests is a huge feat.
Thanks. It's greatly appreciated.

I'll get on to a code review of it shortly.

FYI, the travis checks failed with the following automated linter errors. We'll need to correct them before merging etc.:

src/IRremoteESP8266.h:335:  At least two spaces is best between code and comments  [whitespace/comments] [2]

src/ir_MWM.cpp:16:  At least two spaces is best between code and comments  [whitespace/comments] [2]
src/ir_MWM.cpp:22:  At least two spaces is best between code and comments  [whitespace/comments] [2]
src/ir_MWM.cpp:109:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/ir_MWM.cpp:152:  You don't need a ; after a }  [readability/braces] [4]
src/ir_MWM.cpp:133:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
src/ir_MWM.cpp:133:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
src/ir_MWM.cpp:186:  You don't need a ; after a }  [readability/braces] [4]

test/ir_MWM_test.cpp:80:  Redundant blank line at the end of a code block should be deleted.  [whitespace/blank_line] [3]
test/ir_MWM_test.cpp:147:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
test/ir_MWM_test.cpp:225:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
test/ir_MWM_test.cpp:230:  Redundant blank line at the end of a code block should be deleted.  [whitespace/blank_line] [3]

tools/mode2_decode.cpp:9:  Found C system header after C++ system header. Should be: mode2_decode.h, c system, c++ system, other.  [build/include_order] [4]
tools/mode2_decode.cpp:10:  Found C system header after C++ system header. Should be: mode2_decode.h, c system, c++ system, other.  [build/include_order] [4]
tools/mode2_decode.cpp:68:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
tools/mode2_decode.cpp:68:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]
tools/mode2_decode.cpp:81:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
tools/mode2_decode.cpp:91:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
Total errors found: 18

crankyoldgit

First off, again. wow. I've only got really small nit-pick comments and improvements. Top stuff!

Please add yourself to https://github.com/markszabo/IRremoteESP8266/blob/master/.github/Contributors.md etc. You've earned it!

A few minor house-keeping things to add in addition:

Please add MWM to:
https://github.com/markszabo/IRremoteESP8266/blob/master/src/IRutils.cpp#L131
and
https://github.com/markszabo/IRremoteESP8266/blob/master/src/IRutils.cpp#L172

crankyoldgit · 2018-10-17T14:51:30Z

+  irsend.makeDecodeResult();
+  ASSERT_TRUE(irrecv.decode(&irsend.capture));
+  EXPECT_EQ(MWM, irsend.capture.decode_type);
+  EXPECT_EQ(8*sizeof(short_expected)/sizeof(short_expected[0]), irsend.capture.bits);


Minor style nitpick:

Suggested change

EXPECT_EQ(8*sizeof(short_expected)/sizeof(short_expected[0]), irsend.capture.bits);

EXPECT_EQ(8 * sizeof(short_expected) / sizeof(short_expected[0]), irsend.capture.bits);

crankyoldgit · 2018-10-17T15:01:01Z

@@ -0,0 +1,106 @@
+// Quick and dirty tool to decode GlobalCache (GC) codes


I think the description here doesn't match what the program does, nor who the author is. ;-)
Please update and maybe include a usage example.

My guess from skimming the code is it takes STDIN in the form of lines with "space nnnn" or "pulse nnnn" where nnnn is the duration. One per line etc. Correct?

bwarden · 2018-10-18T03:41:57Z

Thanks for the review! I'm working through the list now; hope to push changes soon. When it's good, I'm happy to squash the commits if you prefer.

crankyoldgit · 2018-10-18T04:19:47Z

I'll end up squashing in the final "Squash and merge" on github. Thanks for getting on to it quickly. Plus, thanks again for writing unit tests. <3 <3 ;-)

For simplicity and maintenance, just stick with the max length for state field

bwarden · 2018-10-18T04:42:18Z

OK, travis is probably failing due to the googletest issues I had locally. I'm not sure how to resolve that; I'm not familiar with submodules or how to revert to the version that was working before my changes.

crankyoldgit · 2018-10-18T13:31:20Z

Yeah, I've never included the lib/googletest suite in any commits before. I'm not sure how sub-modules work either. I'll clone your repo and see what I can work out.

There are a few cpplint.py issues that Travis found that need to be fixed:

src/ir_MWM.cpp:17:  Should have a space between // and comment  [whitespace/comments] [4]
tools/mode2_decode.cpp:96:  Tab found; better to use spaces  [whitespace/tab] [1]
tools/mode2_decode.cpp:107:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 3

crankyoldgit · 2018-10-18T14:43:00Z

Hmm.

git clone https://github.com/bwarden/IRremoteESP8266
cd IRremoteESP8266/test
make install-googletest
make run_tests

Compiles and runs as expected on my machine. Weird.

* Update IRMQTTServer for MWM support. * Fix some cpplint.py found issues. * Adjust the MWM banner. * Anchor the googletest library to v1.8.x to solve c++11 issues.

**[Bug Fixes]** - Add missing send() method to IRPanasonicAC class. (#545) - Add missing sendWhirlpoolAC() to IRMQTTServer.ino (#558) **[Features]** - Add IR receiving support to IRMQTTServer. (#543) - Pioneer support (#547) - Add support for a second LG protocol variant. (#552) - Support for short Panasonic A/C messages. (#553) - Add support for Panasonic CKP series A/Cs. (#554) - Experimental timer/clock support for Panasonic A/Cs. (#546) - Add Made With Magic (MWM) support (#557) **[Misc]** - Grammar and typo fixes (#541, #549) - Increase Panasonic A/C message tolerances. (#542) - Added command mode2_decode in tools/ (#557) - General code style cleanup (#560)

crankyoldgit · 2018-10-21T20:22:20Z

This is now live in the new v2.5.2 release of the library.

bwarden added 10 commits October 16, 2018 21:17

Adding Made With Magic support

4569bac

Adding support for Disney's Made With Magic IR protocol, formerly known as Glow With the Show.

Bump googletest

70d485e

Make mark and space methods public

701059b

Added command mode2_decode needs the mark and space methods from IRsend_test.h to inject events based on received timing data.

Add compiler flags to build googletest

1179bcc

Enforce kStateSizeMax

b84367f

Implement passing send tests for MWM

88d4e87

Implement initial decode test for MWM

45995c6

Different length decode test for MWM

856d42d

Explain origin

3ec992d

crankyoldgit self-requested a review October 17, 2018 14:16

crankyoldgit self-assigned this Oct 17, 2018

crankyoldgit added the enhancement label Oct 17, 2018

crankyoldgit requested changes Oct 17, 2018

View reviewed changes

Incorporate review feedback

7d528af

Revert to kStateSizeMax = kHitachiAc2StateLength

d257a73

For simplicity and maintenance, just stick with the max length for state field

Update status to implemented

f5cf209

crankyoldgit force-pushed the master branch from dc3a62d to e4e75ad Compare October 19, 2018 02:40

crankyoldgit approved these changes Oct 19, 2018

View reviewed changes

Add MWM to MQTT example and fix lint issues.

8859bcc

* Update IRMQTTServer for MWM support. * Fix some cpplint.py found issues. * Adjust the MWM banner. * Anchor the googletest library to v1.8.x to solve c++11 issues.

crankyoldgit force-pushed the master branch from e4e75ad to 8859bcc Compare October 19, 2018 02:59

crankyoldgit merged commit ec75c50 into crankyoldgit:master Oct 19, 2018

crankyoldgit added the Hacktoberfest Hacktoberfest participation label Oct 19, 2018

crankyoldgit mentioned this pull request Oct 21, 2018

v2.5.2 release #561

Merged

Navigation Menu

Repository navigation

Add MWM support - #557

Add MWM support#557
crankyoldgit merged 14 commits into
crankyoldgit:masterfrom
bwarden:master

bwarden commented Oct 17, 2018

Uh oh!

crankyoldgit commented Oct 17, 2018

Uh oh!

crankyoldgit left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crankyoldgit Oct 17, 2018

Uh oh!

Uh oh!

Uh oh!

crankyoldgit Oct 17, 2018

Uh oh!

Uh oh!

bwarden commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 18, 2018

Uh oh!

bwarden commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	EXPECT_EQ(8*sizeof(short_expected)/sizeof(short_expected[0]), irsend.capture.bits);
	EXPECT_EQ(8 * sizeof(short_expected) / sizeof(short_expected[0]), irsend.capture.bits);

		@@ -0,0 +1,106 @@
		// Quick and dirty tool to decode GlobalCache (GC) codes

Conversation

bwarden commented Oct 17, 2018

Uh oh!

crankyoldgit commented Oct 17, 2018

Uh oh!

crankyoldgit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crankyoldgit Oct 17, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

crankyoldgit Oct 17, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bwarden commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 18, 2018

Uh oh!

bwarden commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 18, 2018

Uh oh!

crankyoldgit commented Oct 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants