104 Vehicle interface with ultrasonic sensor#149
Conversation
zhaoshengEE
requested review from
divyabudihal,
hannahvsawiuk,
jackguo380 and
nancyhong123
zhaoshengEE
changed the title
jackguo380
left a comment
jackguo380
left a comment
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.
|
|
||
| //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
hannahvsawiuk
left a comment
hannahvsawiuk
left a comment
There was a problem hiding this comment.
Please make the changes to main.ino I requested
|
@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
| 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
| 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); | ||
4 participants
No description provided.