Skip to content

104 Vehicle interface with ultrasonic sensor - #149

Open
zhaoshengEE wants to merge 6 commits into
masterfrom
104_vehicle_interface_with_sensor
Open

104 Vehicle interface with ultrasonic sensor#149
zhaoshengEE wants to merge 6 commits into
masterfrom
104_vehicle_interface_with_sensor

Conversation

@zhaoshengEE

Copy link
Copy Markdown
Collaborator

No description provided.

@zhaoshengEE zhaoshengEE changed the title Vehicle interface with ultrasonic sensor 104 Vehicle interface with ultrasonic sensor Mar 23, 2020

@jackguo380 jackguo380 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment thread vehicle_interface/RC_with_US/README.md Outdated
@@ -0,0 +1,10 @@
# PropBot

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice! thanks for adding this

Comment thread vehicle_interface/RC_with_US/main.ino Outdated
//////////////////////////////////////////////////////////
// Code Example: Read the sensor at the default address //
//////////////////////////////////////////////////////////
int read_the_sensor_example(){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this used anywhere?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this function is called on the 1st line of the loop function.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you rename it so it doesn't have example maybe read_ultrasonic_sensor()

@divyabudihal

Copy link
Copy Markdown
Collaborator

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

Comment thread vehicle_interface/main/UCMotor.cpp Outdated
return currentstep;
}

/**

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

these are the same files, i am unsure of why it is appearing as changed

Comment thread vehicle_interface/main/main.ino Outdated
//////////////////////////////////////////////////////////
// Code Example: Read the sensor at the default address //
//////////////////////////////////////////////////////////
int read_the_sensor_example(){

@hannahvsawiuk hannahvsawiuk Mar 27, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nice

/**
* @brief fetches RC commands and returns motor commands

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

hmmm I would rebase from master. I believe this is outdated

@hannahvsawiuk hannahvsawiuk left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please make the changes to main.ino I requested

@hannahvsawiuk

Copy link
Copy Markdown
Owner

@zhaoshengEE I think it would be better if we had seperate dirs for the different modules. Once you make the changes to main.ino I will update the structure

Comment thread vehicle_interface/main/main.ino Outdated
void loop()
{
int distance = read_the_sensor_example();
if (distance > 25){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

make this value a define somewhere

Comment thread vehicle_interface/main/main.ino Outdated
void loop()
{
int distance = read_the_sensor_example();
if (distance > 25){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just change this to if (ultrasonic_sensor_range() > MAX_RANGE or smth like that

Comment thread vehicle_interface/main/rc_commands.h Outdated
/* Function prototypes for rc commands */
Array<wheel_motor_command_t, NUM_WHEELS> fetch_rc_commands(void);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

newline

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.

4 participants