Skip to content

Bootcamp: Richard Mao - #325

Open
aris-q wants to merge 1 commit into
UWARG:masterfrom
aris-q:master
Open

Bootcamp: Richard Mao#325
aris-q wants to merge 1 commit into
UWARG:masterfrom
aris-q:master

Conversation

@aris-q

@aris-q aris-q commented Sep 11, 2026

Copy link
Copy Markdown

No description provided.

@EemanAleem EemanAleem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • 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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The 1000 value must be bigger. Also again, nit: magic nums need vars

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants