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

Added extra spare pin to P1 (P1S6) with GPIO and PWM support. #1120

Merged
merged 9 commits into from
Sep 20, 2016

Conversation

technobly
Copy link
Member

@technobly technobly commented Sep 19, 2016

Added extra spare pin to P1 (P1S6) with GPIO and PWM support. Implements #1059

This is the TESTMODE pin 33 on the P1 module, and to use it properly the Wi-Fi Powersave Clock must be disabled. See docs below.


Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation (for 0.6.1-rc.1) [Docs] [P1 Datasheet]

Docs: particle-iot/docs@c63c4f0
P1 Datasheet: particle-iot/docs@0231de8 and particle-iot/docs@486e02b

  • Add to CHANGELOG.md after merging (add links to docs and issues)

FEATURES

  • [P1] Added extra spare pin to P1 (P1S6) with GPIO and PWM support. Implements #1059

@technobly technobly added this to the 0.7.x milestone Sep 19, 2016
Copy link
Contributor

@m-mcgowan m-mcgowan left a comment

Choose a reason for hiding this comment

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

All looks good with a quick read over.

@@ -314,6 +319,11 @@ void HAL_Core_Config(void)
#if PLATFORM_ID==8 // Additional pins for P1
for (pin_t pin=24; pin<=29; pin++)
HAL_Pin_Mode(pin, INPUT);
const uint8_t* data = (const uint8_t*)dct_read_app_data(DCT_RADIO_FLAGS_OFFSET);
uint8_t current = (*data);
if ((current&3) == 0x2) {
Copy link
Contributor

@m-mcgowan m-mcgowan Sep 20, 2016

Choose a reason for hiding this comment

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

I know this is my (shoddy!) code, and felt the same way when I wrote it....it's probably best to pull these magic numbers into a function like

bool isWiFiPowersaveClockDisabled() {
 const uint8_t* data = (const uint8_t*)dct_read_app_data(DCT_RADIO_FLAGS_OFFSET);
  uint8_t current = (*data);
  return ((current&3) == 0x2);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

See commit b3f8160 for resurrection of a fairy ;-)

@@ -29,15 +29,26 @@

uint8_t pwm_pins[] = {

#if defined(STM32F2XX)
#if (PLATFORM_ID == 0) // Core
Copy link
Contributor

Choose a reason for hiding this comment

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

lovely! I'm surprised we didn't hit the need to specify the PWM pins per platform before :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I don't know why, but we were not testing all PWM pins on all platforms before this change. I was pleased to see it pass first time on all platforms though! :)

@technobly
Copy link
Member Author

Here's also the result of the power comparison when running the TEST=wiring/no_fixture test while measuring current on VUSB, with powersave clock enabled and disabled. As seen there are no notable differences.
powersave

@m-mcgowan m-mcgowan merged commit d0d9c19 into develop Sep 20, 2016
@m-mcgowan m-mcgowan deleted the feature/testmode_pin branch September 27, 2016 16:43
@technobly technobly modified the milestones: 0.7.x, 0.6.1 Nov 29, 2016
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.

2 participants