Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

New board Discovery L475VG IOT#97

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
VVESTM merged 11 commits intostm32duino:masterfromunknown repositorySep 13, 2017
Merged

New board Discovery L475VG IOT#97

VVESTM merged 11 commits intostm32duino:masterfromunknown repositorySep 13, 2017

Conversation

@ghost
Copy link

@ghostghost commentedSep 4, 2017•
edited by ghost
Loading

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

Copy link
Contributor

@LMESTMLMESTM left a comment

Choose a reason for hiding this comment

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

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

@fpistmfpistm self-requested a reviewSeptember 4, 2017 13:01
@ghost
Copy link
Author

@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.

Copy link
Contributor

@LMESTMLMESTM left a comment

Choose a reason for hiding this comment

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

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

staticuint8_tUSBD_HID_Init (USBD_HandleTypeDef*pdev,
uint8_tcfgidx)
{
UNUSED(cfgidx);
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

RCC_OscInitStruct.PLL.PLLN=40;
RCC_OscInitStruct.PLL.PLLP =7;
RCC_OscInitStruct.PLL.PLLQ =4;
RCC_OscInitStruct.PLL.PLLR =4;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

//SPI definitions
//define 16 channels. As many channel as digital IOs
#defineSPI_CHANNELS_NUM16
#defineSPI_CHANNELS_NUMDEND
Copy link
Contributor

Choose a reason for hiding this comment

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

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

extern"C" {
#endif

#include"clock.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

@ghostghostSep 4, 2017•
edited by ghost
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

OK@fpistm is the boss :-)

Copy link
Member

Choose a reason for hiding this comment

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

Boss_Baby
But I can be wrong...

@fpistmfpistm added enhancementNew feature or request new variantAdd support of new bard labelsSep 4, 2017
@fpistmfpistm added this to theNext release milestoneSep 4, 2017
// {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
Copy link
Member

Choose a reason for hiding this comment

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

why not allowing ADC on D10?

// {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
Copy link
Member

Choose a reason for hiding this comment

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

ditto for D4

// {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
Copy link
Member

Choose a reason for hiding this comment

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

ditto for D7

// {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
Copy link
Member

Choose a reason for hiding this comment

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

ditto D12

// {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
Copy link
Member

Choose a reason for hiding this comment

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

ditto D11

//*** PWM ***

#ifdefHAL_TIM_MODULE_ENABLED
constPinMapPinMap_PWM[]= {
Copy link
Member

Choose a reason for hiding this comment

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

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

#defineUSBD_MANUFACTURER_STRING "Unknown"
#endif/* USBD_VID */
#ifdefUSBD_USE_HID_COMPOSITE
#defineUSBD_HID_PRODUCT_FS_STRING "HID in FS Mode"
Copy link
Member

Choose a reason for hiding this comment

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

#defineLED1 LED_BUILTIN
#defineLED2 16
#defineLED3 41
#defineLED4 LED3
Copy link
Member

Choose a reason for hiding this comment

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

Is LED4 definition really needed?

Copy link
Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

In conclusion do we keep it?

Copy link
Member

Choose a reason for hiding this comment

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

Yes as the name is used.


//SPI definitions
//define 16 channels. As many channel as digital IOs
#defineSPI_CHANNELS_NUM PEND
Copy link
Member

Choose a reason for hiding this comment

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

This change should be populated to all variants ?

Copy link
Author

Choose a reason for hiding this comment

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

Yes it should. I will update the other variants.

Copy link
Member

@fpistmfpistmSep 6, 2017•
edited
Loading

Choose a reason for hiding this comment

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

Pleaser use NUM_DIGITAL_PINS instead of PEND

#defineSS3 8
#defineMOSI 11
#defineMISO 12
#defineSCLK 13
Copy link
Member

Choose a reason for hiding this comment

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

Remove SCLK definition
#define SCK 13
Refer toebe46ff

{PA_6,TIM16,STM_PIN_DATA_EXT(STM_MODE_AF_PP,GPIO_PULLUP,GPIO_AF14_TIM16,1,0)},// TIM16_CH1 - D12/SPI_MISO
// {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
Copy link
Member

Choose a reason for hiding this comment

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

Only one PA_7 should be define

#ifdefHAL_ADC_MODULE_ENABLED
constPinMapPinMap_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
Copy link
Member

Choose a reason for hiding this comment

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

Then It misses the Ax definition in variant.*

@VVESTMVVESTM self-requested a reviewSeptember 6, 2017 08:42
@VVESTM
Copy link
Contributor

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 withd24a022 and3af2d2a

Vincent

@ghost
Copy link
Author

Hi@VVESTM

Thank you. Merge done.

fpr added8 commitsSeptember 12, 2017 15:39
Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
…75E-IOT01/Applications/USB_DeviceSigned-off-by: fpr <fabien.perroquin@wi6labs.com>
Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
…SB Audio class)Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
@fpistm
Copy link
Member

Think to update the Readme.md with the new board

Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
@ghost ghost requested a review fromfpistmSeptember 13, 2017 11:32
Copy link
Member

@fpistmfpistm left a comment

Choose a reason for hiding this comment

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

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

Signed-off-by: fpr <fabien.perroquin@wi6labs.com>
@ghost ghost requested a review fromfpistmSeptember 13, 2017 12:39
@VVESTMVVESTM merged commit1ae377b intostm32duino:masterSep 13, 2017
@ghost ghost deleted the DISCO_L475VG_IOT branchSeptember 19, 2017 08:08
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@VVESTMVVESTMAwaiting requested review from VVESTM

Assignees

No one assigned

Labels

enhancementNew feature or requestnew variantAdd support of new bard

Projects

None yet

Milestone

Next release

Development

Successfully merging this pull request may close these issues.

3 participants

@VVESTM@fpistm@LMESTM
[8]ページ先頭
©2009-2025 Movatter.jp