Bootcamp: Richard Mao - #325
Conversation
EemanAleem
left a comment
There was a problem hiding this comment.
Good progress so far, but needs some changes!
| MX_SPI1_Init(); | ||
| MX_TIM1_Init(); | ||
| /* USER CODE BEGIN 2 */ | ||
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 1); |
There was a problem hiding this comment.
You need to start the timer before you write anything to the pin!
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 1); | ||
| HAL_TIM_PWM_Start(&htim1, TIM_CHANNEL_1); | ||
| uint8_t adc_TxData[3] = {0x01, 0x80, 0x00}; | ||
| uint8_t adc_RxData[3] = {0x00, 0x00, 0x00}; |
There was a problem hiding this comment.
Either comment or define as variables those bit values. They are correct! But make sure you don't have any magic numbers; part of good coding practice.
| MX_SPI1_Init(); | ||
| MX_TIM1_Init(); | ||
| /* USER CODE BEGIN 2 */ | ||
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 1); |
There was a problem hiding this comment.
You also need to use variables for what you're writing -- inthis case, I'm assuming you mean to use GPIO_PIN_RESET. Never just directly write values unless it's 100% clear what they mean, and even then, it could be misinterpreted by other developers.
| /* USER CODE END WHILE */ | ||
|
|
||
| /* USER CODE BEGIN 3 */ | ||
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 0); |
There was a problem hiding this comment.
See 2nd comment on line 96
|
|
||
| /* USER CODE BEGIN 3 */ | ||
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 0); | ||
| HAL_SPI_TransmitReceive(&hspi1, adc_TxData, adc_RxData, 3, HAL_MAX_DELAY); |
There was a problem hiding this comment.
- 3 is the length of the array, so use that instead of 3 directly
- Use the value received by the HAL_SPI_TransmitReceive function. It is there to tell you whether the operation succeeded or not, so print it in order to let the developer (you) know whether it's working.
| /* USER CODE BEGIN 3 */ | ||
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 0); | ||
| HAL_SPI_TransmitReceive(&hspi1, adc_TxData, adc_RxData, 3, HAL_MAX_DELAY); | ||
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 1); |
There was a problem hiding this comment.
See 2nd comment on line 96.
| HAL_GPIO_WritePin(cs_Port, cs_Pin, 1); | ||
|
|
||
| // find the adc value by only keeping the last 10 bits | ||
| uint16_t comb = (adc_RxData[1] << 8) | adc_RxData[2]; |
There was a problem hiding this comment.
need to mask certain bits in adc_RxData[1]. the mask below is unnecessary as it just masks everything.
| uint16_t adc_value = comb & mask; | ||
|
|
||
| // use linear mapping. Take the range 0-1023 of adc_value and map to 1000-2000 for on time of PWM | ||
| uint16_t mapped_value = (int)(adc_value * 1000.0 / 1023) + 1000; |
There was a problem hiding this comment.
The 1000 value must be bigger. Also again, nit: magic nums need vars
No description provided.