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

Add support for the decapitated final badge. #206

Merged
merged 1 commit into from
Aug 15, 2017
Merged

Conversation

basvs
Copy link
Contributor

@basvs basvs commented Aug 14, 2017

Copy link
Contributor

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Can't really test but looks reasonable to me. Would be cool if we could convince Travis to build with each possible SHA_BADGE_REV value, but oh well.

@@ -140,6 +140,40 @@
#define MPR121_PIN_NUM_LEDS 10
#define MPR121_PIN_NUM_SD_CD 11

#elif defined(CONFIG_SHA_BADGE_V3_LITE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Too bad about the duplication but adding logic would make it harder to follow, so 👍

Copy link
Member

Choose a reason for hiding this comment

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

That should be do-able. .

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, the question is just if it'd be an improvement: removing the duplication will make the code (slightly) less straightforward.

I'm OK with removing the duplication, but it's not necessary IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we see it as a different board at the moment, I think code duplication isn't really a problem. To ease some things, we could split the badge_pins.h in a badge_config_v1.h, badge_config_v2.h, badge_config_v3.h and badge_config_v3_lite.h. Then we can use diff to spot the differences between these files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

optionally prefix the '#include "badge_config_v3.h"' with a '#define HAVE_BADGE_MPR121' or '#undef HAVE_BADGE_MPR121'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I wanted to rename badge_pins.h to badge_config.h a long time ago... ;) )

@basvs basvs merged commit a276bd9 into master Aug 15, 2017
@basvs basvs deleted the basvs-decapitated-badge branch August 15, 2017 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants