Fix #126: Add system clock calibration and read analog code from Catena-Arduino-Platform module #127
Conversation
…na-Arduino-Platform module
chwon64
requested review from
svelmurugan92,
terrillmoore and
dhineshkumarmcci
| @@ -33,17 +33,25 @@ Copyright notice and license information: | |||
|
|
|||
| #ifdef USBCON | |||
|
|
|||
| #define ANALOG_CHANNEL_VBUS 2 | |||
| #if defined(ARDUINO_MCCI_CATENA_4610) || defined(ARDUINO_MCCI_CATENA_4611) | |||
There was a problem hiding this comment.
4611 also non-rechargeable battery variant. I think it required READ_COUNT 6
There was a problem hiding this comment.
We also need same change in C-A-P . Catena 4611 use ReadCount==6 for reading VBUS.
|
I just realized user have to upgrade Catena-Arduino-Platform change first which is mcci-catena/Catena-Arduino-Platform#217. |
cores/arduino/board.h
Outdated
| @@ -23,6 +23,9 @@ extern "C"{ | |||
| #ifdef USBCON | |||
| #include "usb_interface.h" | |||
| #endif //USBCON | |||
| #include "stm32_rtc.h" | |||
There was a problem hiding this comment.
Do we need these included everywhere? Maybe better to include these in the .c files. Not needed for API definitions as far as I can see.
There was a problem hiding this comment.
Okay, I will remove these files from board.h.
cores/arduino/stm32/stm32_adc.cpp
Outdated
| /* the clock we'll use: */ | ||
| constexpr uint32_t AdcClockMode = AdcClockModeAsync; | ||
|
|
||
| #ifdef __cplusplus |
There was a problem hiding this comment.
Since this is a .cpp file, I don't think we need this -- it's always defined.
| extern "C" { | ||
| #endif | ||
|
|
||
| #ifdef STM32L0xx |
There was a problem hiding this comment.
Is this the right sentinel variable? I think we should be using an ARDUINO_ARCH_... variable or something like that.
There was a problem hiding this comment.
We have STM32L0xx and ARDUINO_STM32L0xx. But other stm32 code use STML0xx, STM32L1xx, … If you want to use ARDUINO_STM32L0xx, I can use it.
cores/arduino/stm32/stm32_adc.cpp
Outdated
| *pValue = 2; | ||
|
|
||
| // make sure the clock is configured | ||
| // ADC1->CFGR2 = (ADC1->CFGR2 & ~ADC_CFGR2_CKMODE) | AdcClockMode; |
There was a problem hiding this comment.
do we need this commented-out code?
cores/arduino/stm32/stm32_adc.cpp
Outdated
| { | ||
| ADC1->CR &= ~ADC_CR_ADEN; | ||
|
|
||
| // if (! AdcDisable()) |
cores/arduino/stm32/stm32_adc.cpp
Outdated
| } | ||
| } | ||
|
|
||
| // uint32_t calData = ADC1->DR; |
cores/arduino/stm32/stm32_clock.cpp
Outdated
|
|
||
| if (CalibNew != Calib) | ||
| { | ||
| Serial.print(CalibNew < Calib ? '+' : '-'); |
There was a problem hiding this comment.
do we want this print in production?
There was a problem hiding this comment.
I will comment out. There is another prints and I will comment out too.
| return vBus > 250 ? 1 : 0; | ||
| } | ||
| vBus = Stm32ReadAnalog(ANALOG_CHANNEL_VBUS, READ_COUNT, MULTIPLIER); | ||
| return vBus < 3000 ? 0 : 1; |
| USBD_LL_ConnectionState_WEAK uint32_t USBD_LL_ConnectionState(void) | ||
| { | ||
| uint32_t vBus; | ||
| uint32_t USBD_LL_ConnectionState(void) |
There was a problem hiding this comment.
I wonder if we should have a static variable and only do an analog input for this purpose once every second or two. Spinning up the clock consumes a lot of power and time.
There was a problem hiding this comment.
Okay, I will add code as suggested.
No description provided.