Skip to content

Fixed pinouts of Red and Green led of MKR 1010 wifi led example #1124

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ngolf
Copy link

@ngolf ngolf commented Jun 30, 2023

What This PR Changes

This swaps the pinouts of the Red and Green led. I believe the current example has them wrong (at least produces the wrong colours on my board). It also seems intuitive that the pinout order matches "R G B"

Contribution Guidelines

ngolf added 4 commits June 23, 2023 10:57
I believe the pin colour assimnments in this tutorial were wrong (at least my MKR-WiFi-1010 doesn't match them). Green and Red seem to be swapped around.

This code changes swaps the red and green pins.

It also seems to make sense that the pin order matches the colour order in "RGB"
Update built-in-rgb.md for pin colour assignments
@jhansson-ard jhansson-ard requested a review from jacobhylen July 10, 2023 08:09
@jhansson-ard jhansson-ard added the community Bugs and fixes suggested by the community label Jul 10, 2023
@seaxwi
Copy link
Contributor

seaxwi commented Jul 17, 2023

@ngolf Are you sure you're using a MKR WiFi 1010 board? If so, can you share the sketch that is producing the wrong colors?

The full sketch in the tutorial produces the expected behavior on my board (GREEN, RED, BLUE, OFF):

#include <WiFiNINA.h>
#include <utility/wifi_drv.h>

void setup() {
  WiFiDrv::pinMode(25, OUTPUT); //define green pin
  WiFiDrv::pinMode(26, OUTPUT); //define red pin
  WiFiDrv::pinMode(27, OUTPUT); //define blue pin
}

void loop() {
  WiFiDrv::analogWrite(25, 255);
  WiFiDrv::analogWrite(26, 0);
  WiFiDrv::analogWrite(27, 0);

  delay(1000);

  WiFiDrv::analogWrite(25, 0);
  WiFiDrv::analogWrite(26, 255);
  WiFiDrv::analogWrite(27, 0);

  delay(1000);

  WiFiDrv::analogWrite(25, 0);
  WiFiDrv::analogWrite(26, 0);
  WiFiDrv::analogWrite(27, 255);

  delay(1000);

  WiFiDrv::analogWrite(25, 0);
  WiFiDrv::analogWrite(26, 0);
  WiFiDrv::analogWrite(27, 0);

  delay(1000);
}

Here's a reference to the unintuitive pin ordering by an Arduino firmware engineer: arduino-libraries/WiFiNINA#24 (comment).

I think this example could be improved by defining constants for the RGB pins and using them inside the loop() function.

@seaxwi
Copy link
Contributor

seaxwi commented Jul 17, 2023

Here's a reworked example that (1) declaring pin constants and (2) uses the conventional RGB order when referencing the pins:

#include <WiFiNINA.h>
#include <utility/wifi_drv.h>

const uint8_t LED_RED = 26;
const uint8_t LED_GREEN = 25;
const uint8_t LED_BLUE = 27;


void setup() {
  WiFiDrv::pinMode(LED_RED, OUTPUT); //define red pin
  WiFiDrv::pinMode(LED_GREEN, OUTPUT); //define green pin
  WiFiDrv::pinMode(LED_BLUE, OUTPUT); //define blue pin
}

void loop() {
  WiFiDrv::analogWrite(LED_RED, 255);
  WiFiDrv::analogWrite(LED_GREEN, 0);
  WiFiDrv::analogWrite(LED_BLUE, 0);

  delay(1000);

  WiFiDrv::analogWrite(LED_RED, 0);
  WiFiDrv::analogWrite(LED_GREEN, 255);
  WiFiDrv::analogWrite(LED_BLUE, 0);

  delay(1000);

  WiFiDrv::analogWrite(LED_RED, 0);
  WiFiDrv::analogWrite(LED_GREEN, 0);
  WiFiDrv::analogWrite(LED_BLUE, 255);

  delay(1000);

  WiFiDrv::analogWrite(LED_RED, 0);
  WiFiDrv::analogWrite(LED_GREEN, 0);
  WiFiDrv::analogWrite(LED_BLUE, 0);

  delay(1000);
}

@ngolf
Copy link
Author

ngolf commented Jul 17, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Bugs and fixes suggested by the community maker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants