-
Notifications
You must be signed in to change notification settings - Fork 420
Bootcamp: Mohamed Bekdach #269
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
nucleof072rb/Core/Src/main.c
Outdated
| // convert resolution to # of counts | ||
| uint16_t counts = adcValToCounts(resolution); | ||
| // have the timer start | ||
| HAL_TIM_Base_Start(&htim1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Close, but you need to use the PWM version of this function.
Also the timer should have started before the event loop of the program
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this, thank you
nucleof072rb/Core/Src/main.c
Outdated
| for(int i = 0; i < sizeof(TRANSMISSION_DATA); ++i){ | ||
| send_buf[i] = TRANSMISSION_DATA[i]; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed it, thank you
nucleof072rb/Core/Src/main.c
Outdated
| /* USER CODE BEGIN 0 */ | ||
|
|
||
| uint16_t adcValToCounts(uint16_t resolution){ | ||
| return (resolution / maxADCVal) * maxCounterVal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This formula doesn't linearly scale between 5 and 10 percent of your chosen value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if the formula is now correct
nucleof072rb/Core/Src/main.c
Outdated
|
|
||
| /* USER CODE BEGIN PV */ | ||
| const char TRANSMISSION_DATA[3] = {0x01, 0x80, 0x00}; // 00000001 10000000 (SGL/DIFF = 1, CH0 config) 00000000 | ||
| const uint16_t maxCounterVal = 2400; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have a max counter val be 2400, but there is an optimal solution we're looking for in terms of prescalar and period values.
Also looking at your commit history it seems like any changes you made to the timer peripheral weren't saved since there's no diff for the .ioc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, let me know if my scaling function is correct, then I will look into the optimal solution in terms of prescalar and period values. Also pushed all the STM files
nucleof072rb/Core/Src/main.c
Outdated
|
|
||
| /* USER CODE BEGIN PV */ | ||
| const uint16_t maxCounterVal = 2400; | ||
| const uint16_t minOnPeriodCounterVal = 2160; // This is 10% off of 2400, so the on period would be 2160-2400 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic would actually be the other way around, so from 0-2160 the pwm signal would be high.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok, so is it just the comment and variable name that are incorrect?
nucleof072rb/Core/Src/main.c
Outdated
| /* USER CODE BEGIN 0 */ | ||
|
|
||
| uint16_t adcValToCounts(uint16_t resolution){ | ||
| return (resolution / maxADCVal) * (maxCounterVal - minOnPeriodCounterVal) + minOnPeriodCounterVal; // scale value up to a range of 2160-2400 (10% duty cycle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is good
nucleof072rb/Core/Src/main.c
Outdated
| HAL_SPI_TransmitReceive(&hspi1, (uint8_t*)&send_buf, (uint8_t*)&recv_buf, 3, 100); | ||
| HAL_GPIO_WritePin(GPIOB, GPIO_PIN_8, GPIO_PIN_SET); | ||
|
|
||
| uint16_t resolution = ( ((uint16_t)recv_buf[2] << 8) & (uint16_t)recv_buf[3]) & 0x0003FF; // 10 bit value of ADC, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would revisit the bitwise logic for this line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, should be an OR there, not AND.
nucleof072rb/Core/Src/main.c
Outdated
| void runMotor(uint16_t resolution) { | ||
| // convert resolution to # of counts | ||
| uint16_t counts = adcValToCounts(resolution); | ||
| // if it reaches the value we have in the resolution -> motor pin gets set to high |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic actually works the other way around
No description provided.