Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Two minor fixes, add missing include and provide pin definitions for A0 to A7#122

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
hlovdal wants to merge3 commits intoArduino-CI:master
base:master
Choose a base branch
Loading
fromhlovdal:ostream_and_pin

Conversation

hlovdal
Copy link
Contributor

No description provided.

@ianfixes
Copy link
Collaborator

I googled the name of those pins and found a file from@PaulStoffregen :
https://github.com/PaulStoffregen/cores/blob/master/teensy3/pins_arduino.h

Is there a larger effort in defining pins that we need to make here? (I won't delay merging this, I'll just open some issues)

@hlovdal
Copy link
ContributorAuthor

Teensy appears to be aArduino compatible ARM Coretex board with 34 I/O pins. So most proper Arduino hardware targets will have less, but I do not feel that there is any problem in overcommitting on providing possible non-existing pin definitions in the test environment. I will add and rebase the branch.

@hlovdal
Copy link
ContributorAuthor

One question though. The pins have different numerical values depending on which hardware target, and some of them are conflicting, e.g.A20 is either 31 or 39 in thepins_arduino.h linked above.
And this is also the case for other pin definitions like

#definePIN_WIRE_SDA         (2)

in~/.arduino15/packages/arduino/hardware/avr/1.6.209/variants/leonardo/pins_arduino.h vs

#definePIN_WIRE_SDA        (18)

in~/.arduino15/packages/arduino/hardware/avr/1.6.209/variants/standard/pins_arduino.h

Do we need to keep a 1-to-1 consistency with all hardware targets? That will be a lot of work, or could I just create an enum with consecutive values (possibly starting on a non-compatible value like 100 just to avoid the situation that some of the pin definition are the same as the real hardware while some are not, and at least be consistent in that none are)).

Copy link
Collaborator

@ianfixesianfixes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm still figuring out how this review thing works. I thought you would have seen these comments 7 hours ago with my other comment, but apparently not.

@@ -1,3 +1,5 @@
#include <ostream>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm assuming that I was breaking compilation (somewhere) without this. Can you describe where the trouble was?

@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased]
### Added

- Provide pin definitions for A0 to A7.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

But only on some platforms, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Most platforms have such definitions. Maybe this can be controlled using aNUM_ANALOG_PINS (which would be good to define in any case, along withNUM_DIGITAL_PINS, since those are at least defined for AVR-based boards:https://github.com/rlcamp/ArduinoCore-avr/blob/4c9f899f8eef12989903e474beea3a71a90f511c/variants/standard/pins_arduino.h#L28-L29)

@ianfixes
Copy link
Collaborator

It sounds like you understand my hesitation here, and I think you're asking the right questions.

Do we need to keep a 1-to-1 consistency with all hardware targets?

No. We just need to make sure that the mocks will work if you stay consistent (i.e. if within the library we refer toA20 and within the unittest we do something likeassertEqual(A20, myLib->somePin), it should always work, and not give a31 =/= 39 assertion error).

I do not feel that there is any problem in overcommitting on providing possible non-existing pin definitions in the test environment

It depends. One of the features offered by this library is the ability to unittest across Arduino hardware platforms, so I need to be somewhat careful that the test environments are accurate. (E.g. a library would break if it was doing#ifdef A20 to determine available hardware features.) I doubt that people are usingA20 in particular for their macros, so that's probably fine. But that's how I'm approaching this.

The sort of pattern I'm trying to support is this one:https://github.com/adafruit/Adafruit-WS2801-Library/blob/master/Adafruit_WS2801.h#L59

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@matthijskooijmanmatthijskooijmanmatthijskooijman left review comments

@ianfixesianfixesianfixes left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@hlovdal@ianfixes@matthijskooijman

[8]ページ先頭

©2009-2025 Movatter.jp