-
Notifications
You must be signed in to change notification settings - Fork 39
Resolve warning and compile errors with examples. #61
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @konkers thanks for the PR! I left a few comments to work on before I accept. Also cool testing setup - I haven't yet played witht he arduino-cli.
@@ -16,8 +17,14 @@ | |||
#define EULER 2.718281828459045235360287471352 | |||
|
|||
//Special "pads" used by the ADC to select the special internal ADC channels | |||
#ifndef ADC_DIFF0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These codes used to be board dependent but are not any longer, so should not be defined by the user. They should always evaluate to the values shown here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cores/arduino/ard_sup/Arduino.h
Outdated
@@ -83,4 +82,6 @@ extern "C" | |||
|
|||
#include "variant.h" | |||
|
|||
#include "Arduino_defines.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of this move?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to accommodate the overriding the values in the variant file. Since that is no longer needed, neither is this.
@@ -38,9 +38,11 @@ void loop() | |||
{ | |||
digitalWrite(LED, LOW); | |||
|
|||
#ifdef A3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a user selects a board without A3 (perhaps it uses A23 or some other alias -- who knows, it is up to the board designer what to call pins) this will cause the example to compile and simply drop a desired feature without a warning. In this case I believe a warning to be acceptable. It would signal to the user "your board does not have A3" as explained in the comments at the top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was approaching this from a testing standpoint (verses an education one) where the rest of the example still had testing value and I wanted to fail tests on warnings. I disable Example4 for in my test harness for boards that do not have A3.
@@ -22,6 +22,11 @@ SOFTWARE. | |||
#ifndef _AP3_VARIANT_H_ | |||
#define _AP3_VARIANT_H_ | |||
|
|||
// Define these pins before including Arduino.h so that they can overide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before -- the method used here has changed. Instead of adding ADC_DIFF0 we should just remove all the variant-specific ADC codes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Updated PR incoming once I verify it passes tests. |
These were not referenced in by the SPI library.
All comments should be addressed and updated now. |
Awesome, thanks a ton! |
Resolves all warning and compile errors on all examples on all boards except those listed below. Tested with test harness at: https://github.com/konkers/Arduino_Apollo3/commit/efdb5489437e18e90c24f1e9c3e11e1e5bf416ea
Tests skipped
libraries/Examples/examples/Example2_Serial:
libraries/PDM/examples/Example1_MicrophoneOutput:
libraries/PDM/examples/Example2_ConfigureMic:
libraries/PDM/examples/Example3_FullConfigure:
libraries/Servo/examples/Example6_19servos:
libraries/Servo/examples/Example7_29servos: