IDE 1.5.3 bugs / issues

darrylp · September 3, 2013, 10:28am

for the avr core wiring_analog.c

how is the following bit of code any good as compared to 1.5.2 ?

analogPinToChannel is not defined for standard and mega, in fact its only defined in variants/lenardo and the robot_control [& motor]
the double check and action for the ATmega32U4 is so good ! not !!!

int analogRead(uint8_t pin)
{
	uint8_t low, high;

#if defined(analogPinToChannel)
#if defined(__AVR_ATmega32U4__)
	if (pin >= 18) pin -= 18; // allow for channel or pin numbers
#endif
	pin = analogPinToChannel(pin);
#elif defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)
	if (pin >= 54) pin -= 54; // allow for channel or pin numbers
#elif defined(__AVR_ATmega32U4__)
	if (pin >= 18) pin -= 18; // allow for channel or pin numbers
#elif defined(__AVR_ATmega1284__) || defined(__AVR_ATmega1284P__) || defined(__AVR_ATmega644__) || defined(__AVR_ATmega644A__) || defined(__AVR_ATmega644P__) || defined(__AVR_ATmega644PA__)
	if (pin >= 24) pin -= 24; // allow for channel or pin numbers
#else
	if (pin >= 14) pin -= 14; // allow for channel or pin numbers
#endif

darrylp · September 3, 2013, 12:02pm

and another similar bug in HardwareSerial.cpp for the avr

  #if defined(UDR0)
    if (bit_is_clear(UCSR0A, UPE0)) {
      unsigned char c = UDR0;
      store_char(c, &Serial);
    } else {
      unsigned char c = UDR0;
    };
  #elif defined(UDR)
    if (bit_is_clear(UCSRA, PE)) {
      unsigned char c = UDR;
      store_char(c, &rx_buffer);    <----------  rx_buffer doesn't exist, think its meant to be &Serial here !
    } else {
      unsigned char c = UDR;
    };
  #else
    #error UDR not defined
  #endif

drjiohnsmith · September 3, 2013, 4:36pm

are u surprised ?

the arduino team will say its beta,

but..

darrylp · September 4, 2013, 9:00am

drjiohnsmith:
are u surprised ?

the arduino team will say its beta,

but..

no of course not surprised, just thought someone who is on the developer list might actually watch in here. 1.5.4 is going to need the two things fixing as otherwise its breaking existing code base. first with the commented out pin number adapting, affects all boards except lenardo and robot ones, and the other is for processors that have UDR defined instead of UDR0 ( no idear what they are Attiny maybe ? )

cmaglie · September 9, 2013, 10:07am

@darryl

thanks for finding the HardwareSerial error.

About the analogPinToChannel, I don't understand if you find another bug?
You can find some discussion about the patch here:

github.com/arduino/Arduino

Allow analogPinToChannel for chips other than ATTinyX5

opened 04:04PM - 16 Apr 13 UTC

closed 10:06AM - 21 Feb 14 UTC

tarunlightup

For my board, it makes sense to renumber the pins on the ATTiny84. Thus, the ADC… channel is no longer the same number as the Arduino pin number. The macro "analogPinToChannel" seems to address this, but it currently is only called for ATTinyX5 and ATMega32U4. I'm able to get my board working if I edit the arduino core (analogRead function) to ensure that the macro is called without restriction if defined. Could this macro be allowed to work for all chips without restriction if defined, instead of being restricted to only be called for ATTinyX5 and ATMega32U4? It would make it easier to use other chips without having to update the analogRead function each time.

darrylp · September 9, 2013, 3:42pm

cmaglie:
@darryl

thanks for finding the HardwareSerial error.

About the analogPinToChannel, I don't understand if you find another bug?
You can find some discussion about the patch here:

Allow analogPinToChannel for chips other than ATTinyX5 · Issue #1366 · arduino/Arduino · GitHub
Use analogPinToChannel if it's defined by Lauszus · Pull Request #1368 · arduino/Arduino · GitHub

as per the discussion in the two links, the non defining of "analogPinToChannel" blocks out the code in the first message of this thread, for example any code accessing the ADC pins of a Mega 2560, will no longer work.

the code is alos badly formed, follow the trace for example of a Due ( which has AVR_ATmega32U4 defined ? )

you will see its taking off 18 twice from the pin number....

here is the code again, but indented to show the nesting clearer

#if defined(analogPinToChannel)
    #if defined(__AVR_ATmega32U4__)
        if (pin >= 18) pin -= 18; // allow for channel or pin numbers
    #endif
	pin = analogPinToChannel(pin);
#elif defined(__AVR_ATmega1280__) || defined(__AVR_ATmega2560__)
	if (pin >= 54) pin -= 54; // allow for channel or pin numbers
#elif defined(__AVR_ATmega32U4__)
	if (pin >= 18) pin -= 18; // allow for channel or pin numbers
#elif defined(__AVR_ATmega1284__) || defined(__AVR_ATmega1284P__) || defined(__AVR_ATmega644__) || defined(__AVR_ATmega644A__) || defined(__AVR_ATmega644P__) || defined(__AVR_ATmega644PA__)
	if (pin >= 24) pin -= 24; // allow for channel or pin numbers
#else
	if (pin >= 14) pin -= 14; // allow for channel or pin numbers
#endif

I dont use a arduino board with a processor that defines the following.... AVR_ATmega32U4 but I expect it wont be what it is expected if you dont use the low numbers to accessthe ADC pins.

cmaglie · September 9, 2013, 4:22pm

darryl:
as per the discussion in the two links, the non defining of "analogPinToChannel" blocks out the code in the first message of this thread, for example any code accessing the ADC pins of a Mega 2560, will no longer work.

@darryl
Sorry, but I still can't understand where the issue is, may you provide an example that fails?

I tried to compile for Mega2560 and the compiler preprocessor selected:

 if (pin >= 54) pin -= 54;

as expected.

darryl:
the code is alos badly formed, follow the trace for example of a Due ( which has AVR_ATmega32U4 defined ? )

The Due doesn't have AVR_ATmega32U4 defined, and use a totally different wiring_analog.c, you should look here for the Due:

github.com

arduino/Arduino/blob/ide-1.5.x/hardware/arduino/sam/cores/arduino/wiring_analog.c

/*
 Copyright (c) 2011 Arduino.  All right reserved.

 This library is free software; you can redistribute it and/or
 modify it under the terms of the GNU Lesser General Public
 License as published by the Free Software Foundation; either
 version 2.1 of the License, or (at your option) any later version.

 This library is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 See the GNU Lesser General Public License for more details.

 You should have received a copy of the GNU Lesser General Public
 License along with this library; if not, write to the Free Software
 Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 */

#include "Arduino.h"

This file has been truncated. show original

darrylp · September 9, 2013, 7:36pm

cmaglie:

darryl:
as per the discussion in the two links, the non defining of "analogPinToChannel" blocks out the code in the first message of this thread, for example any code accessing the ADC pins of a Mega 2560, will no longer work.

@darryl
Sorry, but I still can't understand where the issue is, may you provide an example that fails?

I tried to compile for Mega2560 and the compiler preprocessor selected:
 if (pin >= 54) pin -= 54;
as expected.

darryl:
the code is alos badly formed, follow the trace for example of a Due ( which has AVR_ATmega32U4 defined ? )

The Due doesn't have AVR_ATmega32U4 defined, and use a totally different wiring_analog.c, you should look here for the Due:

Arduino/hardware/arduino/sam/cores/arduino/wiring_analog.c at ide-1.5.x · arduino/Arduino · GitHub

Oh dear, I can now see after sitting and studying the code, not using my default editor, that a bug in the editor is highlighting and code matching incorrectly.
I feel so silly !
It has however shown a problem that the nesting of c/c++ defines and conditionals is now broken ! just not the arduino code, but my editor !

mentioning the due, I of course meant the lenardo ( as per the first post )
so please forgive me trying to convince you of some bad code.... at least the second post had a real bug !

sorry again !

Lauszus · September 9, 2013, 8:40pm

@darryl
The reason why the pin number gets subtracted by 18 when you are using a Leonardo is because I didn't want to break backwards compatibility with third party versions of the Leonardo. Take a look at the original code before my pull request: Arduino/wiring_analog.c at aba27c43aa1d4ea827c8a07774e7ea398cd7b259 · arduino/Arduino · GitHub, as you can see it subtracts 18 before analogPinToChannel is called.

I know that I could just have removed it and then fixed it in the variants file: Arduino/pins_arduino.h at ide-1.5.x · arduino/Arduino · GitHub.
That would have worked just fine with the official Leonardo, but would have broken analog reading on third party boards using a ATmega32U4 with it's own variants file.

If you still thinks it's a bug, then fell free to open up another pull request that removes this, but I doubt it will get merged because of it's incompatibility with existing third party boards.

darrylp · September 9, 2013, 10:20pm

Lauszus:
@darryl
The reason why the pin number gets subtracted by 18 when you are using a Leonardo is because I didn't want to break backwards compatibility with third party versions of the Leonardo. Take a look at the original code before my pull request: Arduino/hardware/arduino/avr/cores/arduino/wiring_analog.c at aba27c43aa1d4ea827c8a07774e7ea398cd7b259 · arduino/Arduino · GitHub, as you can see it subtracts 18 before analogPinToChannel is called.

I know that I could just have removed it and then fixed it in the variants file: Arduino/hardware/arduino/avr/variants/leonardo/pins_arduino.h at ide-1.5.x · arduino/Arduino · GitHub.
That would have worked just fine with the official Leonardo, but would have broken analog reading on third party boards using a ATmega32U4 with it's own variants file.

If you still thinks it's a bug, then fell free to open up another pull request that removes this, but I doubt it will get merged because of it's incompatibility with existing third party boards.

no, the code is good. I was reading what my IDE editor was showing me as code to be used for my compile options, rather than what the gcc pre-processor would actually use. it has a bug, that i will look into tomorrow when back at work

cmaglie · September 10, 2013, 7:40am

darryl:
so please forgive me trying to convince you of some bad code.... at least the second post had a real bug !

No worries!

Topic		Replies	Views
Analog Pin Reporting in Latest IDE Suggestions for the Arduino Project	13	4031	May 6, 2021
who can explain this code ???? thx Programming	12	63183	May 5, 2021
definitions and analog pins General Guidance	8	3562	May 5, 2021
PORTC Bugs & Suggestions	7	3060	May 6, 2021
Platform macro? Programming	4	802	May 5, 2021

IDE 1.5.3 bugs / issues

Related topics