Skip to content
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

Read sensors in the systick #263

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Read sensors in the systick #263

wants to merge 3 commits into from

Conversation

Peque
Copy link
Member

@Peque Peque commented Apr 24, 2018

Fixes #216.

@Peque Peque added this to the Portugal 2018 milestone Apr 24, 2018
@Peque Peque self-assigned this Apr 24, 2018
@Peque Peque requested a review from cua-cua April 24, 2018 18:42
@Peque Peque force-pushed the sensors branch 2 times, most recently from 30c5d97 to 111f8b8 Compare April 25, 2018 20:37

disable_walls_control();
side_sensors_calibration();
Copy link
Member

Choose a reason for hiding this comment

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

why are you using side_sensors_calibration?

Copy link
Member Author

@Peque Peque Apr 25, 2018

Choose a reason for hiding this comment

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

Removed in 58693b9. 👍

sleep_us(1000);
}
required_deceleration =
(.3 * .3) /
Copy link
Member

Choose a reason for hiding this comment

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

Macro for set_target_linear_speed and these .3* .3.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created a variable to store that value (58693b9).

}
}

/**
* @brief Get sensors values with emitter on and off.
* @brief Update the sensors raw readings.
Copy link
Member

Choose a reason for hiding this comment

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

Change the description to be different than on the function "update_raw_readings". Include the "Clear injected end of conversion" comment on the header of the function. Does not exist a function on libopencm3 to do that? (as for timer floag: timer_clear_flag(TIM1, TIM_SR_UIF))

Copy link
Member

Choose a reason for hiding this comment

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

You can use adc_clear_flag from common.c file (pull request to libopencm3, it was already working for watchdog interruption flag)

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems the adc_clear_flag() differs from what it is written here, right? Now we have:

ADC_SR(ADC1) &= ~ADC_SR_JEOC; 

But calling adc_clear_flag() would result in:

ADC_SR(ADC1) = ~ADC_SR_JEOC; 

Note the & is missing.

Is adc_clear_flag() broken?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed docstring as requested: 6faa22a

}

/**
* @brief Update the sensors raw readings.
Copy link
Member

Choose a reason for hiding this comment

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

Expand the description

@@ -290,7 +279,7 @@ void side_sensors_calibration(void)
for (i = 0; i < SIDE_CALIBRATION_READINGS; i++) {
left_temp += distance[SENSOR_SIDE_LEFT_ID];
right_temp += distance[SENSOR_SIDE_RIGHT_ID];
sleep_ticks(SENSORS_SM_TICKS);
sleep_ticks(2);
Copy link
Member

Choose a reason for hiding this comment

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

why did you change it to 2? Coudl you add an explicative macro?

Copy link
Member Author

@Peque Peque Oct 8, 2018

Choose a reason for hiding this comment

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

How would you call it? It is just some small delay in between readings, it could be any other number really.

I could create a SIDE_CALIBRATION_READINGS_DELAY_TICKS but looks huge just for one use and it really is not important. Does it hurt too much in your eyes? 😂

diff = 0;
else
diff = on - off;
diff /= 4;
Copy link
Member

Choose a reason for hiding this comment

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

Why 4?

src/control.c Outdated
@@ -9,14 +9,14 @@
*/
static volatile float linear_acceleration = 5.;
Copy link
Member

Choose a reason for hiding this comment

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

why these changes on this commit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just for now, while the robot keeps crashing into walls like a drunken robot... I will remove it before merging into master. I hope...

@@ -143,6 +143,7 @@ void run_movement_sequence(const char *sequence)
*/
void run_front_sensors_calibration(void)
Copy link
Member

Choose a reason for hiding this comment

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

good :)

Copy link
Member Author

Choose a reason for hiding this comment

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

😄

@Peque Peque force-pushed the sensors branch 4 times, most recently from 5fb877a to bf359f5 Compare April 27, 2018 17:39
@Peque Peque changed the title Read sensors in the systick WIP: Read sensors in the systick Apr 27, 2018
@Peque Peque added the wip label Apr 27, 2018
@Peque Peque modified the milestones: Portugal 2018, OSHWDem 2018 May 1, 2018
@Peque Peque removed the enhancement label May 1, 2018
@Peque Peque requested a review from cua-cua October 8, 2018 19:42
@Peque Peque added enhancement and removed wip labels Oct 8, 2018
@Peque Peque changed the title WIP: Read sensors in the systick Read sensors in the systick Oct 8, 2018
This was referenced Oct 11, 2018
@Peque
Copy link
Member Author

Peque commented Oct 27, 2018

Moved to Portugal 2019 milestone. Added "on hold" label as this method results in less reliable readings. We probably need some hardware changes to improve that (i.e.: add capacitors next to the emitters to provide the required transient power).

@Peque Peque removed the enhancement label Oct 27, 2018
@Peque Peque removed the request for review from cua-cua October 27, 2018 01:16
@Peque Peque modified the milestones: Portugal 2019, OSHWDem 2019 May 16, 2019
@Peque Peque modified the milestones: OSHWDem 2019, Portugal 2020 Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants