Quick and dirty tool to decode GC codes #308
Conversation
There was a problem hiding this comment.
First of all, wow, thanks for doing this. That's really useful. I apologize in advance for my feedback/comments/suggestions. They are mostly minor nitpicks and style guide things to make things look/read better later on.
One other nitpick/suggestion, could you use a 'tools' directory instead of 'tool' I'm sure we will collect more down the track. Might as well name it correctly this time around. ;-)
Basically, other than the nitpicks, it looks good. We really should only have one copy of IRsend_test.h though. :-/
tool/IRsend_test.h
Outdated
| @@ -0,0 +1,104 @@ | |||
| // Copyright 2017 David Conran | |||
|
|
|||
| #ifndef TEST_IRSEND_TEST_H_ | |||
There was a problem hiding this comment.
Can you reference the canonical file instead of making essentially a copy of it here? It would be better if you just included the "real" IRsend_test.h file instead of making a copy which we need to keep in sync etc.
tool/Makefile
Outdated
| COMMON_DEPS = $(USER_DIR)/IRrecv.h $(USER_DIR)/IRsend.h $(USER_DIR)/IRtimer.h \ | ||
| $(USER_DIR)/IRutils.h $(USER_DIR)/IRremoteESP8266.h | ||
| # Common test dependencies | ||
| COMMON_TEST_DEPS = $(COMMON_DEPS) IRsend_test.h |
There was a problem hiding this comment.
per the other comment, perhaps ../test/IRsend_test.h instead? or what ever makes it use the original file.
tool/gc_decode.cpp
Outdated
| @@ -0,0 +1,62 @@ | |||
| #include "IRsend.h" | |||
| #include "IRsend_test.h" | |||
There was a problem hiding this comment.
Per other comments. Lets use the original file.
Also, the ordering of the headers I think needs to be different per styleguide.
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
tool/gc_decode.cpp
Outdated
|
|
||
|
|
||
| int main (int argc, char * argv[]) { | ||
|
|
There was a problem hiding this comment.
Remove duplicate blank lines. https://google.github.io/styleguide/cppguide.html#Vertical_Whitespace
tool/gc_decode.cpp
Outdated
|
|
||
|
|
||
| if (argc != 2) { | ||
| cout << "Use gc_decode [global_code]" << endl; |
There was a problem hiding this comment.
Indenting misaligned. pls fix.
tool/gc_decode.cpp
Outdated
| char * pch; | ||
| pch = strtok (argv[1],","); | ||
| while (pch != NULL) | ||
| { |
There was a problem hiding this comment.
Bracer to the end of the previous line. (style guide thing)
tool/gc_decode.cpp
Outdated
|
|
||
| char * pch; | ||
| pch = strtok (argv[1],","); | ||
| while (pch != NULL) |
There was a problem hiding this comment.
You need a bounds check on the size of gc_test so you don't go writing data outside the array.
e.g.
while (pch != NULL && index < 250) {
tool/gc_decode.cpp
Outdated
| return 0; | ||
| } | ||
|
|
||
| uint16_t gc_test[250]; |
There was a problem hiding this comment.
Nitpick/suggestion: Probably best to put the 250 value into a #define etc, and make it larger. e.g. 512 or something. Some GC codes can be very long.
tool/gc_decode.cpp
Outdated
| irrecv.decode(&irsend.capture); | ||
|
|
||
| cout << "Code GC length " << index << endl; | ||
| cout << "Code type " << irsend.capture.decode_type << endl; |
There was a problem hiding this comment.
You should probably also print out the common name for the type. e.g. See the switch statement in the dumpV2 example code.
tool/gc_decode.cpp
Outdated
| cout << "Code command " << std::hex << irsend.capture.command << endl; | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Can you please run the Google cpplint.py over this code? https://github.com/cpplint/cpplint
Also, can you please add it to the travis.yaml file so a) it gets a "does it compile" check, and b) update the cpplint.py call/args in the travis.yaml file to also cover code in this directory, so it gets linted etc. ;-)
|
Oh, and please feel free to add yourself to the contributers file etc. If you want. ;-) |
Added #define to control the max length for the GC code. Now show the type of code as string Fix errors found using cpplint Removed unused headers
|
Hi David I think that I did all the changes that you requested, as I said, my code was something that I did very quickly and I know that was very dirty. I did the change to travis.yml too. If there anything else please let me know. by the way, this library is awesome I used it on my hardware. |
|
Looks great to me! And a big thanks for adding to the project and doing this. ;-) |
I created a tool to decode a GC code, the code is quick and dirty but I think that can be used as the base to create something better.
Example Power on for Roku