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

[Electron] Optional feature to use HSE/LSI as RTC clock source instead of LSE (external 32KHz XTAL) #2354

Closed
wants to merge 7 commits into from

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Sep 10, 2021

This PR currently targets v2.0.1 branch for review, DO NOT MERGE

Description

As title says.

Steps to Test

  • Run wiring/electron_disable_external_rtc_clock and wiring/electron_enable_external_rtc_clock with device-os-test. This can easily be done with electron_disable_external_rtc_clock filter i.e. device-os-test run electron electron_disable_external_rtc_clock
  • Run all the tests after setting System.enableFeature(FEATURE_DISABLE_EXTERNAL_LOW_SPEED_CLOCK) (this is best done through Concourse, the pipeline has been updated to run Electron tests with LSE and with HSE/LSI). The same thing can be done locally with device-os-test --device-os-dir=/path/to/device-os run electron
  • Run any sleep tests after setting System.enableFeature(FEATURE_DISABLE_EXTERNAL_LOW_SPEED_CLOCK)

Example App

void setup() {
    System.enableFeature(FEATURE_DISABLE_EXTERNAL_LOW_SPEED_CLOCK);
    System.disableFeature(FEATURE_DISABLE_EXTERNAL_LOW_SPEED_CLOCK);
    if (System.featureEnabled(FEATURE_DISABLE_EXTERNAL_LOW_SPEED_CLOCK)) {
        //
    }
}

void loop() {

}

References

N/A


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@@ -84,21 +101,187 @@ void HAL_RTC_Initialize_UnixTime()
/* Get calendar_time date struct values */
RTC_DateStructure.RTC_WeekDay = 6;
RTC_DateStructure.RTC_Date = 1;
RTC_DateStructure.RTC_Month = 0;
RTC_DateStructure.RTC_Month = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Just to double confirm that the month starts from 1, since the month field in some RTC chip varies from 0 ~ 11, which is corresponding to month 1 ~ 12.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here, it's actually expected to start with 1. Previously the date ended up being Dec 1999 instead of Jan 2000.


static bool hal_rtc_switch_clock_source(uint32_t source)
{
if ((RCC->BDCR & 0x00000300) != (source & 0x300) && (RCC->BDCR & 0x00000300) != 0x000)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: magic numbers

RTC_DateStructure.RTC_Year = 0;

setRTCTime(&RTC_TimeStructure, &RTC_DateStructure);
}

static void hal_rtc_configure()
Copy link
Member

Choose a reason for hiding this comment

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

Prefer to return something in case of failure to conditionally execute following procedures.

}
}

static void hal_rtc_configure_clock()
Copy link
Member

Choose a reason for hiding this comment

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

Prefer to return something in case of failure to conditionally execute following procedures.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is handled by asserts.


RCC_RTCCLKConfig(source);

hal_rtc_configure();
Copy link
Member

Choose a reason for hiding this comment

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

This may fail, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants