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

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
adds the system feature `FEATURE_WIFI_POWERSAVE_CLOCK` (default enabl…
…ed) which can be used to disable the WiFI powersave clock. The feature setting persisted. Acceptance test application in `user/test/app/testmodepin`
@technobly

This comment has been minimized.

Copy link
Member

commented on hal/src/stm32f2xx/pinmap_hal.c in d6e0669 Aug 23, 2016

Nice @m-mcgowan! TESTMODE just happens to be the actual pin 33 (PA8 on STM) on the P1 module, but the indexes here for PIN_MAP are not related to those pins... so no need for the padding really. I believe one goal for this pin is to use it as a PWM output as well, so that would need to be defined here for TIM1_CH1 (TIM1, TIM_Channel_1,). Also for finishing touches I would suggest adding a P1S6 pin define in hal/inc/pinmap_hal.h.

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

@technobly technobly referenced this pull request Sep 19, 2016
115 of 125 tasks complete
@m-mcgowan
Copy link
Contributor

left a comment

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) {

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Sep 20, 2016

Contributor

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);
}

This comment has been minimized.

Copy link
@technobly

technobly Sep 20, 2016

Author Member

See commit b3f8160 for resurrection of a fairy ;-)

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

uint8_t pwm_pins[] = {

#if defined(STM32F2XX)
#if (PLATFORM_ID == 0) // Core

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Sep 20, 2016

Contributor

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

This comment has been minimized.

Copy link
@technobly

technobly Sep 20, 2016

Author Member

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

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2016

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

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@m-mcgowan m-mcgowan deleted the feature/testmode_pin branch Sep 27, 2016

@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
Projects
None yet
2 participants
You can’t perform that action at this time.