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

Bug (solved): D24 bit value is wrong in digital pin masks. #3129

Closed
wants to merge 1 commit into from

Conversation

rafacouto
Copy link

In Leonardo hardware pin assignment, the correspondence between bit mask and pin PD5 is missing, while PD4 is repeated as D24, so D24 should be PD5.

Atmega32U4 PD5 pin is XCK1/~CTS and it is connected to TX led on Leonardo board. After this patch, D24 is assigned to PD5 and these lines can blink TX led:

void setup() {
  pinMode(24, OUTPUT);
}

void loop() {
 digitalWrite(24, HIGH);   // turn the LED on (HIGH is the voltage level)
 delay(500);              // wait for a second
 digitalWrite(24, LOW);    // turn the LED off by making the voltage LOW
 delay(500); 
}

This bug was found by @xdesig (member of the Escornabot project :) This is his hardware unit test: https://www.youtube.com/watch?v=nClBTrmG7Sk

@NicoHood
Copy link
Contributor

That is correct, D4 is A6 so pin 24 is useless and can be assigned to the other LED which is not used yet.

I'd also vote for a
#define LED_BUILTIN_TX 24
#define LED_BUILTIN_RX 17

@NicoHood NicoHood mentioned this pull request Jul 4, 2015
@NicoHood
Copy link
Contributor

NicoHood commented Aug 6, 2015

Its such a simple fix, why dont you apply this?

@matthijskooijman
Copy link
Collaborator

I don't think this fix is correct. Pin 24 was previously used for (virtual) pin "A6", which is the analog input mode on pin D4. This needs a duplicate pin number to be assigned to pin PD4 (as well as others), because analogRead() is expected to work with both pin numbers and analog channel numbers (e.g. 0 for A0). If you would just say A6 = 4 and then do analogRead(A6), this would cause ambiguity: Did you mean pin 4 (A6) or channel 4 (A4)?

So I believe that D24 must remain unchanged. Adding a pin for the TX led makes sense, though I'm not sure if it would be ok to add it at the end - some code might be assuming that the last NUM_ANALOG_INPUTS are analog pins (though perhaps not). See also #2071, where some similar problems were encountered.

@NicoHood
Copy link
Contributor

NicoHood commented Aug 7, 2015

Oh you are right. But why not use another pin for the led then?

#define LED_BUILTIN_TX 30
#define LED_BUILTIN_RX 17

@cmaglie
Copy link
Member

cmaglie commented Aug 17, 2015

As stated by @matthijskooijman the current pin assignment is correct.
Closing as invalid.

@cmaglie cmaglie closed this Aug 17, 2015
@cmaglie cmaglie added the Type: Invalid Off topic for this repository, or a bug report determined to not actually represent a bug label Aug 17, 2015
@ffissore ffissore added this to the Release 1.6.6 milestone Aug 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture: AVR Applies only to the AVR microcontrollers (Uno, etc.) Component: Core Related to the code for the standard Arduino API Type: Invalid Off topic for this repository, or a bug report determined to not actually represent a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants