Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add F() Macros #670

Merged
merged 5 commits into from
Apr 12, 2019
Merged

Add F() Macros #670

merged 5 commits into from
Apr 12, 2019

Conversation

jimmys01
Copy link
Contributor

No description provided.

Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. That's a lot of effort. :)
You've use String() calls when I think you don't need to. They are to be avoided as they are probably unneeded and they require special handling to allow the code to compile outside of the Arduino Framework. e.g. Unit tests and tools etc. use standard gcc/g++ etc on Linux etc.

src/IRutils.cpp Outdated Show resolved Hide resolved
src/IRutils.cpp Outdated Show resolved Hide resolved
src/IRutils.cpp Outdated Show resolved Hide resolved
src/IRutils.cpp Outdated Show resolved Hide resolved
src/ir_Daikin.cpp Outdated Show resolved Hide resolved
src/ir_Haier.cpp Outdated Show resolved Hide resolved
src/ir_Panasonic.cpp Outdated Show resolved Hide resolved
src/ir_Whirlpool.cpp Outdated Show resolved Hide resolved
src/ir_Gree.cpp Outdated Show resolved Hide resolved
Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis found at least one compilation error:

../src/ir_Daikin.cpp: In member function ‘std::string IRDaikin2::toString()’:
../src/ir_Daikin.cpp:1263:35: error: ‘String’ was not declared in this scope
   result += String(", Swing (H): ") + uint64ToString(getSwingHorizontal());

and a pile-of-lint issues:

$ python cpplint.py --extensions=c,cc,cpp,ino --headers=h,hpp {src,test,tools}/*.{h,c,cc,cpp,hpp,ino} examples/*/*.{h,c,cc,cpp,hpp,ino}

src/ir_Coolix.cpp:378:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:463:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:1231:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:1289:  Tab found; better to use spaces  [whitespace/tab] [1]
src/ir_Daikin.cpp:1330:  Extra space before last semicolon. If this should be an empty statement, use {} instead.  [whitespace/semicolon] [5]
src/ir_Daikin.cpp:1338:  Extra space before last semicolon. If this should be an empty statement, use {} instead.  [whitespace/semicolon] [5]
src/ir_Gree.cpp:379:  Tab found; better to use spaces  [whitespace/tab] [1]
src/ir_Hitachi.cpp:271:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
src/ir_Panasonic.cpp:691:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Tcl.cpp:313:  Extra space before last semicolon. If this should be an empty statement, use {} instead.  [whitespace/semicolon] [5]
src/IRutils.cpp:377:  Tab found; better to use spaces  [whitespace/tab] [1]
src/IRutils.cpp:393:  Tab found; better to use spaces  [whitespace/tab] [1]
src/IRutils.cpp:394:  Tab found; better to use spaces  [whitespace/tab] [1]
src/IRutils.cpp:396:  Tab found; better to use spaces  [whitespace/tab] [1]
src/IRutils.cpp:397:  Tab found; better to use spaces  [whitespace/tab] [1]
src/IRutils.cpp:401:  Tab found; better to use spaces  [whitespace/tab] [1]
src/IRutils.cpp:402:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 17

which need to be fixed.

@jimmys01
Copy link
Contributor Author

Time is up for today, thank you for your guidance

@crankyoldgit
Copy link
Owner

Time is up for today, thank you for your guidance

You've done well so far. That's a lot of effort in those commits. :-)

src/ir_Daikin.cpp Outdated Show resolved Hide resolved
src/ir_Gree.cpp Outdated Show resolved Hide resolved
@crankyoldgit
Copy link
Owner

Only a few linter issues to go:

src/ir_Coolix.cpp:378:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:463:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:1231:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:1331:  Extra space before last semicolon. If this should be an empty statement, use {} instead.  [whitespace/semicolon] [5]
src/ir_Daikin.cpp:1339:  Extra space before last semicolon. If this should be an empty statement, use {} instead.  [whitespace/semicolon] [5]
src/ir_Panasonic.cpp:691:  Extra space after ( in function call  [whitespace/parens] [4]

These two I suspect are incorrect indentation levels. or when you refactored a single line into multiple ones.

src/ir_Daikin.cpp:1288:  If/else bodies with multiple statements require braces  [readability/braces] [4]
src/ir_Gree.cpp:377:  If/else bodies with multiple statements require braces  [readability/braces] [4]

The unit tests now can run though and have detected a number of errors. I'll re-review and try to find what problems have been introduced.

Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few bugs and typos were introduced that need to be fixed before merge.
Other than those and the few remaining lint issues, it's looking great!

src/ir_Haier.cpp Outdated Show resolved Hide resolved
src/ir_Coolix.cpp Outdated Show resolved Hide resolved
src/ir_Mitsubishi.cpp Outdated Show resolved Hide resolved
```
src/ir_Coolix.cpp:378:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:463:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:1231:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:1331:  Extra space before last semicolon. If this should be an empty statement, use {} instead.  [whitespace/semicolon] [5]
src/ir_Daikin.cpp:1339:  Extra space before last semicolon. If this should be an empty statement, use {} instead.  [whitespace/semicolon] [5]
src/ir_Panasonic.cpp:691:  Extra space after ( in function call  [whitespace/parens] [4]
src/ir_Daikin.cpp:1288:  If/else bodies with multiple statements require braces  [readability/braces] [4]
src/ir_Gree.cpp:377:  If/else bodies with multiple statements require braces  [readability/braces] [4]
```

```
ir_Haier_test.cpp:363: Failure
Expected equality of these values:
  "Command: 1 (On), Mode: 0 (AUTO), Temp: 25C, Fan: 0 (AUTO), " "Swing: 0 (Off), Sleep: Off, Health: Off, " "Current Time: 00:00, On Timer: Off, Off Timer: Off"
    Which is: "Command: 1 (On), Mode: 0 (AUTO), Temp: 25C, Fan: 0 (AUTO), Swing: 0 (Off), Sleep: Off, Health: Off, Current Time: 00:00, On Timer: Off, Off Timer: Off"
  haier.toString()
    Which is: "Command: 1 (On), Mode: 0 (AUTO), Temp: 25C, Fan: 0 (AUTO), Swing: 0, Swing: Off), Sleep: Off, Health: Off, Current Time: 00:00, On Timer: Off, Off Timer: Off"
```

```
ir_Coolix_test.cpp:485: Failure
Expected equality of these values:
  "Power: On, Mode: 4 (FAN), Fan: 5 (AUTO), Zone Follow: Off, " "Sensor Temp: Ignored"
    Which is: "Power: On, Mode: 4 (FAN), Fan: 5 (AUTO), Zone Follow: Off, Sensor Temp: Ignored"
  ircoolix.toString()
    Which is: "Power: On, Mode: 4 (FAN), Fan: 5 (AUTO)255C, Zone Follow: Off, Sensor Temp: Ignored"
```

```
ir_Mitsubishi_test.cpp:976: Failure
Expected equality of these values:
  "Power: On (HEAT), Temp: 22C, FAN: SILENT, VANE: AUTO, " "Time: 17:10, On timer: 00:00, Off timer: 00:00, Timer: -"
    Which is: "Power: On (HEAT), Temp: 22C, FAN: SILENT, VANE: AUTO, Time: 17:10, On timer: 00:00, Off timer: 00:00, Timer: -"
  irMitsu.toString()
    Which is: "Power: On (HEAT), Temp: 22C, Fan: SILENT, VANE: AUTO, Time: 17:10, On timer: 00:00, Off timer: 00:00, Timer: -"
```
@crankyoldgit crankyoldgit self-assigned this Apr 12, 2019
@crankyoldgit crankyoldgit merged commit f33450d into crankyoldgit:master Apr 12, 2019
crankyoldgit added a commit that referenced this pull request Apr 30, 2019
_v2.6.0 (20190430)_

**[Bug Fixes]**
- Fixed problem where LG protocol used wrong duty cycle for repeat. (#687)
- Fix checksum calculation for Daikin protocols. (#678)
- Fix the byte array version of sendGree() (#684, #685)
- Fix artificial vs. real state creation on HaierAC. (#668, #671)
- Fix issues caused by having `MQTT_ENABLE` set to false. (#677)
- Fix compile problem when DEBUG is defined. (#673, #674)
- Fix Minor bug with MQTT_ENABLE False condition (#654)

**[Features]**
- Experimental support for DAIKIN216 (ARC433B69) (#690)
- Experimental support for Mitsubishi Heavy Industries A/Cs. (#660, #665, #667)
- Support more features of TCL A/C (#656)
- Add LEGO(TM) Power Functions IR protocol. (#655)
- Add Panasonic AC RKR model & Example (#649)
- DAIKIN/IRDaikinESP overhaul and add Comfort mode support. (#678)
  **WARNING**: Previous `sendDaikin()` calls may not work.
               Please recapture codes or use `kDaikinStateLengthShort` for
               `nbytes` in those calls.
- IRMQTTServer: Move MQTT server and other parameters to WifiManager. (#680)
  **WARNING**: Previous users may need to fully wipe/reset the
               SPIFFS/WifiManager settings by visiting
               `http://<your_esp8266's_ip_address>/reset` prior to or
               after update.
- Add Wifi filtering options to IRMQTTServer. (#679)
- Add advanced aircon/climate functionality to IRMQTTServer (#677)
- Initial prototype of a common interface for all A/Cs. (#664)
- Improve MQTT topic usage for feedback messages. (#663)
- Add multiple independent GPIO sending support via MQTT. (#661)

**[Misc]**
- Adjust kGreeHdrSpace to 4500 (#684, #686)
- Add Home Assistant mqtt climate instructions. (#682)
- Implement htmlEscape() to prevent XSS etc. (#681)
- Add F() Macros (#670)
- Update Daikin2's Cool mode min temp to 18C (#658)
- Change per byte bit-order in Electra protocol. (#648)
- Improve Daikin2 power on/off. (#647)
@crankyoldgit crankyoldgit mentioned this pull request Apr 30, 2019
crankyoldgit added a commit that referenced this pull request Apr 30, 2019
_v2.6.0 (20190430)_

**[Bug Fixes]**
- Fixed problem where LG protocol used wrong duty cycle for repeat. (#687)
- Fix checksum calculation for Daikin protocols. (#678)
- Fix the byte array version of sendGree() (#684, #685)
- Fix artificial vs. real state creation on HaierAC. (#668, #671)
- Fix issues caused by having `MQTT_ENABLE` set to false. (#677)
- Fix compile problem when DEBUG is defined. (#673, #674)
- Fix Minor bug with MQTT_ENABLE False condition (#654)

**[Features]**
- Experimental support for DAIKIN216 (ARC433B69) (#690)
- Experimental support for Mitsubishi Heavy Industries A/Cs. (#660, #665, #667)
- Support more features of TCL A/C (#656)
- Add LEGO(TM) Power Functions IR protocol. (#655)
- Add Panasonic AC RKR model & Example (#649)
- DAIKIN/IRDaikinESP overhaul and add Comfort mode support. (#678)
  **WARNING**: Previous `sendDaikin()` calls may not work.
               Please recapture codes or use `kDaikinStateLengthShort` for
               `nbytes` in those calls.
- IRMQTTServer: Move MQTT server and other parameters to WifiManager. (#680)
  **WARNING**: Previous users may need to fully wipe/reset the
               SPIFFS/WifiManager settings by visiting
               `http://<your_esp8266's_ip_address>/reset` prior to or
               after update.
- Add Wifi filtering options to IRMQTTServer. (#679)
- Add advanced aircon/climate functionality to IRMQTTServer (#677)
- Initial prototype of a common interface for all A/Cs. (#664)
- Improve MQTT topic usage for feedback messages. (#663)
- Add multiple independent GPIO sending support via MQTT. (#661)

**[Misc]**
- Adjust kGreeHdrSpace to 4500 (#684, #686)
- Add Home Assistant mqtt climate instructions. (#682)
- Implement htmlEscape() to prevent XSS etc. (#681)
- Add F() Macros (#670)
- Update Daikin2's Cool mode min temp to 18C (#658)
- Change per byte bit-order in Electra protocol. (#648)
- Improve Daikin2 power on/off. (#647)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants