104 Vehicle interface with ultrasonic sensor #149
Conversation
zhaoshengEE
requested review from
jackguo380,
divyabudihal,
nancyhong123 and
hannahvsawiuk
zhaoshengEE
changed the title
There was a problem hiding this comment.
I won't review the code as its largely the same as #76 but @hannahvsawiuk should look over the changes and just make sure that there aren't any issues.
Good job on the ultrasonic sensors.
| @@ -0,0 +1,10 @@ | |||
| # PropBot | |||
There was a problem hiding this comment.
nice! thanks for adding this
| ////////////////////////////////////////////////////////// | ||
| // Code Example: Read the sensor at the default address // | ||
| ////////////////////////////////////////////////////////// | ||
| int read_the_sensor_example(){ |
There was a problem hiding this comment.
is this used anywhere?
There was a problem hiding this comment.
Yes, this function is called on the 1st line of the loop function.
There was a problem hiding this comment.
can you rename it so it doesn't have example maybe read_ultrasonic_sensor()
|
instead of having a different folder, can we just have a flag somewhere in the code? this diff is hard to look through because it has a lot of new code that is the same as the other stuff |
vehicle_interface/main/UCMotor.cpp
Outdated
| return currentstep; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
these are the same files, i am unsure of why it is appearing as changed
vehicle_interface/main/main.ino
Outdated
| ////////////////////////////////////////////////////////// | ||
| // Code Example: Read the sensor at the default address // | ||
| ////////////////////////////////////////////////////////// | ||
| int read_the_sensor_example(){ |
There was a problem hiding this comment.
Please, do not include any functions aside from setup and loop in main to keep things clean. Please move to a seperate .cpp file with the prototypes in a corresponding .h file. Also better for future unit testing.
There was a problem hiding this comment.
I cannot include those functions in a separate .cpp file. If I do that, the library function(SoftI2CMaster.h) will be crashed. I know this sounds weird, I tried to debug that but it still doesn't work.
| @@ -4,6 +4,11 @@ | |||
| // Pins defined by the UCMotor header | |||
| #include "UCMotor.h" | |||
|
|
|||
| //Pin definitions for the ultrasonic sensor(MB1202) | |||
| #define SCL_PIN 5 //Default SDA is Pin5 PORTC for the UNO -- you can set this to any tristate pin | |||
| #define SCL_PORT PORTC | |||
|
|
||
|
|
||
| /** | ||
| * @brief fetches RC commands and returns motor commands |
There was a problem hiding this comment.
hmmm I would rebase from master. I believe this is outdated
|
@zhaoshengEE I think it would be better if we had seperate dirs for the different modules. Once you make the changes to |
vehicle_interface/main/main.ino
Outdated
| @@ -60,11 +64,80 @@ void sendCommandMini(UC_DCMotor * uc_motor, wheel_motor_command_t command) { | |||
|
|
|||
| void loop() | |||
| { | |||
| int distance = read_the_sensor_example(); | |||
| if (distance > 25){ | |||
There was a problem hiding this comment.
make this value a define somewhere
vehicle_interface/main/main.ino
Outdated
| @@ -60,11 +64,80 @@ void sendCommandMini(UC_DCMotor * uc_motor, wheel_motor_command_t command) { | |||
|
|
|||
| void loop() | |||
| { | |||
| int distance = read_the_sensor_example(); | |||
| if (distance > 25){ | |||
There was a problem hiding this comment.
just change this to if (ultrasonic_sensor_range() > MAX_RANGE or smth like that
vehicle_interface/main/rc_commands.h
Outdated
| /* Function prototypes for rc commands */ | ||
| Array<wheel_motor_command_t, NUM_WHEELS> fetch_rc_commands(void); | ||
No description provided.