New board Discovery L475VG IOT #97

ghost · 2017-09-04T12:31:03Z

Add source files for the variant DISCO_L475VG_IOT.

The following on board peripherals are available:

USB OTG FS (native)
Flash memory (MX25R6435F library required)
Bluetooth V4.1 module (SPBTLE-RF library required)
Microphones (MP34DT01 library required)
Humidity and temperature sensor (HTS221 library required)
Magnetometer (LIS3MDL library required)
Accelerometer and gyroscope (LSM6DSL library required)
Barometer (LPS22HB library required)
Time of flight and gesture detection sensor (VL53L0X library required)

The following on board peripherals will be available later:

WiFi
NFC
Sub-GHz RF

LMESTM

Question: where do the change from commit "Update USB configuration" come from and what are they fixing ?

ghost · 2017-09-04T13:03:07Z

@LMESTM

the change from commit "Update USB configuration" adapt the usb files from the previous commit (original source files from STM32Cube) for the Arduino project.

LMESTM

Few comments and questions but overall looks like a good PR - thanks for it !!

LMESTM · 2017-09-04T12:45:33Z

cores/arduino/stm32/usbd_hid_composite.c

@@ -369,6 +369,7 @@ __ALIGN_BEGIN static uint8_t HID_KEYBOARD_ReportDesc[HID_KEYBOARD_REPORT_DESC_SI
 static uint8_t  USBD_HID_Init (USBD_HandleTypeDef *pdev,
                               uint8_t cfgidx)
 {
+  UNUSED(cfgidx);


I guess it's ok, but why do you remove them ? can you explain in commit message ?

This removed only function parameters not used inside the function. There is no impact on the rest of the code but allow to remove some warning messages at compilation time.

Ok sorry - this is clear enough in the commit title already !

LMESTM · 2017-09-04T12:50:03Z

variants/DISCO_L475VG_IOT/variant.cpp

+  RCC_OscInitStruct.PLL.PLLN            = 40;
+  RCC_OscInitStruct.PLL.PLLP            = 7;
+  RCC_OscInitStruct.PLL.PLLQ            = 4;
+  RCC_OscInitStruct.PLL.PLLR            = 4;


Does the change have an impact on the CPU or main peripheral clocks ?
Typically do we still have CPU running @ 80MHz ?

This modification do not impact the main clock. It's still running at 80MHz.

LMESTM · 2017-09-04T12:53:45Z

variants/DISCO_L475VG_IOT/variant.h

@@ -76,7 +76,7 @@ enum {

 //SPI definitions
 //define 16 channels. As many channel as digital IOs
-#define SPI_CHANNELS_NUM        16
+#define SPI_CHANNELS_NUM        DEND


Can you be more verbose in the commit message concerning this fix ?
Does it only apply to this new board ?

LMESTM · 2017-09-04T13:04:44Z

cores/arduino/wiring.h

@@ -24,6 +24,8 @@
 extern "C" {
 #endif

+#include "clock.h"


Which compilation error does it fix ? And why is this visible only with this new board ?
Arduino.h is where wiring.h is included from and it first includes<chip.h> that should contain necessary defines.
Maybe the error should be corrected somewhere else ... depends on the actual error

It fixes a compilation error in the library SPBTLE-RF because we call wiring.h and without the fix it doesn't find some function of clock.h.
I tried to use Arduino.h but I had more compilation errors due to missing prototypes.
Maybe there is another solution that could be implemented in the library and not in the core. I will continue to look for.

Ok. I also think it is better to solve the library side and find the proper includes from the library

This include is missing in the core and is required.
Wiring.h declare a static function which use GetCurrentMicro() define in clock.h.
Already see this issue in my test_usb branch on my forked repo.

So I will leave it and we will use it as a fix.

OK @fpistm is the boss :-)

But I can be wrong...

fpistm · 2017-09-05T13:23:39Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

+//  {PA_0,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 5, 0)}, // ADC2_IN5 - D1/UART4_TX
+//  {PA_1,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 6, 0)}, // ADC1_IN6 - D0/UART4_RX
+//  {PA_1,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 6, 0)}, // ADC2_IN6 - D0/UART4_RX
+//  {PA_2,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 7, 0)}, // ADC1_IN7 - D10/PWM


why not allowing ADC on D10?

fpistm · 2017-09-05T13:23:51Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

+//  {PA_1,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 6, 0)}, // ADC2_IN6 - D0/UART4_RX
+//  {PA_2,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 7, 0)}, // ADC1_IN7 - D10/PWM
+//  {PA_2,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 7, 0)}, // ADC2_IN7 - D10/PWM
+//  {PA_3,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 8, 0)}, // ADC1_IN8 - D4


ditto for D4

fpistm · 2017-09-05T13:24:02Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

+//  {PA_2,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 7, 0)}, // ADC2_IN7 - D10/PWM
+//  {PA_3,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 8, 0)}, // ADC1_IN8 - D4
+//  {PA_3,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 8, 0)}, // ADC2_IN8 - D4
+//  {PA_4,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 9, 0)}, // ADC1_IN9 - D7


ditto for D7

fpistm · 2017-09-05T13:24:36Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

+//  {PA_4,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 9, 0)}, // ADC2_IN9 - D7
+//  {PA_5,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 10, 0)}, // ADC1_IN10 - D13/SPI1_SCK/LED1
+//  {PA_5,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 10, 0)}, // ADC2_IN10 - D13/SPI1_SCK/LED1
+//  {PA_6,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 11, 0)}, // ADC1_IN11 - D12/SPI_MISO


fpistm · 2017-09-05T13:24:52Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

+//  {PA_5,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 10, 0)}, // ADC2_IN10 - D13/SPI1_SCK/LED1
+//  {PA_6,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 11, 0)}, // ADC1_IN11 - D12/SPI_MISO
+//  {PA_6,   ADC2,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 11, 0)}, // ADC2_IN11 - D12/SPI_MISO
+//  {PA_7,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 12, 0)}, // ADC1_IN12 - D11/SPI1_MOSI/PWM


fpistm · 2017-09-05T13:27:44Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

+//*** PWM ***
+
+#ifdef HAL_TIM_MODULE_ENABLED
+const PinMap PinMap_PWM[] = {


same for ADC, why PWM could not be available for D4, D13, D12, D14, D15, ...?

fpistm · 2017-09-05T13:36:02Z

variants/DISCO_L475VG_IOT/usb/usbd_desc.c

+#define USBD_MANUFACTURER_STRING "Unknown"
+#endif /* USBD_VID */
+#ifdef USBD_USE_HID_COMPOSITE
+#define USBD_HID_PRODUCT_FS_STRING        "HID in FS Mode"


Consider to add the product name in the string. See: https://github.com/stm32duino/Arduino_Core_STM32/blob/master/variants/NUCLEO_F429ZI/usb/usbd_desc.c#L72

fpistm · 2017-09-05T13:40:59Z

variants/DISCO_L475VG_IOT/variant.h

+#define LED1                    LED_BUILTIN
+#define LED2                    16
+#define LED3                    41
+#define LED4                    LED3


Is LED4 definition really needed?

LED4 is defined to match with the documentation and the PCB serigraphy.

Yes I've saw it. one refers to WiFI and the other to BLE. depending of the gpio state.

In conclusion do we keep it?

Yes as the name is used.

fpistm · 2017-09-05T13:41:50Z

variants/DISCO_L475VG_IOT/variant.h

+
+//SPI definitions
+//define 16 channels. As many channel as digital IOs
+#define SPI_CHANNELS_NUM        PEND


This change should be populated to all variants ?

Yes it should. I will update the other variants.

Pleaser use NUM_DIGITAL_PINS instead of PEND

fpistm · 2017-09-05T13:44:18Z

variants/DISCO_L475VG_IOT/variant.h

+#define SS3                     8
+#define MOSI                    11
+#define MISO                    12
+#define SCLK                    13


Remove SCLK definition
#define SCK 13
Refer to ebe46ff

fpistm · 2017-09-06T07:04:36Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

 //  {PA_6,   TIM3,   STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF2_TIM3, 1, 0)},    // TIM3_CH1 - D12/SPI_MISO
-//  {PA_7,   TIM17,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF14_TIM17, 1, 0)},  // TIM17_CH1 - D11/SPI1_MOSI/PWM
+    {PA_7,   TIM17,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF14_TIM17, 1, 0)},  // TIM17_CH1 - D11/SPI1_MOSI/PWM


Only one PA_7 should be define

fpistm · 2017-09-06T07:05:59Z

variants/DISCO_L475VG_IOT/PeripheralPins.c

@@ -40,25 +40,25 @@

 #ifdef HAL_ADC_MODULE_ENABLED
 const PinMap PinMap_ADC[] = {
-//  {PA_0,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 5, 0)}, // ADC1_IN5 - D1/UART4_TX
+    {PA_0,   ADC1,  STM_PIN_DATA_EXT(STM_MODE_ANALOG, GPIO_NOPULL, 0, 5, 0)}, // ADC1_IN5 - D1/UART4_TX


Then It misses the Ax definition in variant.*

VVESTM · 2017-09-11T07:29:36Z

Hi,
I have executed our test plan on this pull request. All tests are OK, good.

To reduce again the number of commits, a merge can be done with d24a022 and 3af2d2a

Vincent

ghost · 2017-09-11T08:05:16Z

Hi @VVESTM

Thank you. Merge done.

Signed-off-by: fpr <[email protected]>

…75E-IOT01/Applications/USB_Device Signed-off-by: fpr <[email protected]>

Signed-off-by: fpr <[email protected]>

…SB Audio class) Signed-off-by: fpr <[email protected]>

Signed-off-by: fpr <[email protected]>

fpistm · 2017-09-12T13:49:31Z

Think to update the Readme.md with the new board

Signed-off-by: fpr <[email protected]>

fpistm

Ok. Please update the Readme.md with the new board then it could be merged.

Signed-off-by: fpr <[email protected]>

LMESTM reviewed Sep 4, 2017

View reviewed changes

fpistm self-requested a review September 4, 2017 13:01

LMESTM reviewed Sep 4, 2017

View reviewed changes

fpistm added enhancement New feature or request new variant Add support of new bard labels Sep 4, 2017

fpistm added this to the Next release milestone Sep 4, 2017

fpistm reviewed Sep 5, 2017

View reviewed changes

fpistm reviewed Sep 6, 2017

View reviewed changes

VVESTM self-requested a review September 6, 2017 08:42

fpr added 8 commits September 12, 2017 15:39

Add variant DISCO-L475VG-IOT

4d2c0d5

Signed-off-by: fpr <[email protected]>

Add USB OTG FS device files from STM32Cube_FW_L4_V1.8.0/Projects/B-L4…

de14752

…75E-IOT01/Applications/USB_Device Signed-off-by: fpr <[email protected]>

Update USB configuration

b2b0e7a

Signed-off-by: fpr <[email protected]>

Remove some unused parameter warnings

91b06e8

Signed-off-by: fpr <[email protected]>

Add QSPI pin map and indent commented lines

f194ad1

Signed-off-by: fpr <[email protected]>

Enable DFSDM module because used by MP34DT01 library

adfbb3c

Signed-off-by: fpr <[email protected]>

Update USB configuration to allow to use the USBMicrophone library (U…

dd63aec

…SB Audio class) Signed-off-by: fpr <[email protected]>

Update USB descriptor to allow to use USBMicrophone library

ed1ddd8

Signed-off-by: fpr <[email protected]>

Update variant configuration files

949ffce

Signed-off-by: fpr <[email protected]>

fpistm approved these changes Sep 12, 2017

View reviewed changes

Add new function in SPI class required by SPBTLE library

574d1b5

Signed-off-by: fpr <[email protected]>

ghost requested a review from fpistm September 13, 2017 11:32

fpistm reviewed Sep 13, 2017

View reviewed changes

Update README.md

8a5dce8

Signed-off-by: fpr <[email protected]>

ghost requested a review from fpistm September 13, 2017 12:39

fpistm approved these changes Sep 13, 2017

View reviewed changes

VVESTM merged commit 1ae377b into stm32duino:master Sep 13, 2017

ghost deleted the DISCO_L475VG_IOT branch September 19, 2017 08:08

pierremolinaro mentioned this pull request Jan 12, 2021

Cannot use PG9 as digital pin with NUCLEO-H743ZI2 #1276

Closed

fpistm mentioned this pull request Jun 6, 2023

gh pr list do not show some PR cli/cli#7538

Closed

New board Discovery L475VG IOT #97

New board Discovery L475VG IOT #97

Conversation

ghost commented Sep 4, 2017 • edited by ghost Loading

LMESTM left a comment

Choose a reason for hiding this comment

ghost commented Sep 4, 2017

LMESTM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Sep 4, 2017 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpistm Sep 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VVESTM commented Sep 11, 2017

ghost commented Sep 11, 2017

fpistm commented Sep 12, 2017

fpistm left a comment

Choose a reason for hiding this comment

ghost commented Sep 4, 2017 •

edited by ghost

Loading

ghost Sep 4, 2017 •

edited by ghost

Loading

fpistm Sep 6, 2017 •

edited

Loading