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

Fix pin number in Daikin example #383

Merged
merged 1 commit into from
Dec 22, 2017
Merged

Conversation

quozl
Copy link
Contributor

@quozl quozl commented Dec 22, 2017

src/main.cpp:33:22: error: 'D2' was not declared in this scope
IRDaikinESP daikinir(D2);  // An IR LED is controlled by GPIO pin 4 (D2)
                     ^

Compiled on PlatformIO 3.4.1, tested on an Adafruit Feather Huzzah, with correct response from a Daikin A/C.

Note: also occurs in TurnOnKelvinatorAC.ino, TurnOnMitsubishiAC.ino, TurnOnToshibaAC.ino, and TurnOnTrotecAC.ino but I'm unable to test them. 😁 Is the change correct, and shall I widen the change to cover these?

@crankyoldgit
Copy link
Owner

According to https://github.com/esp8266/Arduino/blob/master/variants/nodemcu/pins_arduino.h#L42
static const uint8_t D2 = 4;
i.e. D2, as printed on the silkscreen on the board) is pin 4 technically.

If we are going to change it, we need to set it to 4 or change the comment to D4 if we change the value to 2.

However, I'm more worried why your platformio environment isn't picking up the D4 constant from the above mentioned file. I didn't have trouble compiling them the last time I tried on a Atom/PlatformIO config on a linux machine. (I didn't get that error) I wonder if it is something odd with the feather/huzzah board setup. If so, we should probably change it to something that works for everything.

@crankyoldgit crankyoldgit self-requested a review December 22, 2017 06:12
@quozl
Copy link
Contributor Author

quozl commented Dec 22, 2017

Interesting, thanks. Yes, it was the board setup. D2 is a nodemcu-specific pin designator, perhaps considered an extension to Arduino.

My platformio.ini had

[env:huzzah]
platform = espressif8266
board = huzzah
framework = arduino

~/.platformio/packages/framework-arduinoespressif8266/variants/adafruit/pins_arduino.h doesn't have what the nodemcu path has.

Changing platformio.ini to board = nodemcu fixes the compile error.

Pin marking silkscreen on the Feather Huzzah is 2, and is GPIO2 on schematic, from pin 11 of ESP-12, and it does toggle properly.

I guess it wasn't clear from TurnOnArduinoAC.ino that it was nodemcu-specific. 😁

Happy for you to close if you are targeting nodemcu boards only.

For interest, I'm building on Ubuntu 16.04.

@crankyoldgit
Copy link
Owner

Adafruit are the ones being a bit a-typical. Referring to pins via their labels is supported on traditional Arduino boards for seemingly ever. e.g. Uno, Leonardo, etc.

That said, lets try to make this work for everyone.

In https://github.com/markszabo/IRremoteESP8266/blob/master/examples/IRsendDemo/IRsendDemo.ino#L36
We use just 4. Let's change all of them to be the same 4 on all the "sending" examples.

We can either drop any mention of D2 or expand the comment(s) to say 4 is D2 on NodeMCU/Wemos D1s etc, and 4 on Feathers/Huzzahs etc.

We use just a pin number, and say that pin 4 is D2 on NodeMCU boards.
@quozl
Copy link
Contributor Author

quozl commented Dec 22, 2017

I've just checked, and Arduino IDE with an Uno doesn't accept int led = D13;.

Otherwise, what you propose sounds fine. a4c16ce.

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.

LGTM

@crankyoldgit crankyoldgit self-assigned this Dec 22, 2017
@crankyoldgit crankyoldgit merged commit e69b1f9 into crankyoldgit:master Dec 22, 2017
@quozl quozl deleted the d2 branch December 22, 2017 21:58
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