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

Erick motor pair class #9

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Erick motor pair class #9

wants to merge 8 commits into from

Conversation

LinnierGames
Copy link
Member

@LinnierGames LinnierGames commented Dec 31, 2016

ref #1
closes #1

Additions

  • MotorPair class : initializes two motor instances, moveForward method

@@ -0,0 +1,5 @@
[.ShellClassInfo]
Copy link
Member

Choose a reason for hiding this comment

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

This file should not be source controlled

@@ -0,0 +1,13 @@

Copy link
Member

Choose a reason for hiding this comment

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

This file should not be source controlled

return ceil((min(x,1) - in_min) * (out_max - out_min) / (in_max - in_min) + out_min);
}

class motor
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent capitalization.
Either all class names should be capitalized or none of them should.


void setSpeed(float speed) {

try {
Copy link
Member

Choose a reason for hiding this comment

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

It is generally not best practice to use exceptions to catch bugs (eg. out of bounds speed).
Asserts or something similar (since it is running on a teensy) are much faster and perform the same function and can easily be disabled in a release build.
Exceptions are meant for when the error is out of the programmers control eg. failed to allocate memory, failed to open file, etc.


private:
float speed;
int FORWARD_PIN;
Copy link
Member

Choose a reason for hiding this comment

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

all caps is convention for a constant variable. Pins should be const

}

motor(int set_forward_pin, int set_backward_pin, int set_pwm_pin) {
FORWARD_PIN = set_forward_pin;
Copy link
Member

Choose a reason for hiding this comment

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

Use a member initializor list so that XXX_PIN can be made const

throw speedRangeOutOfBounds();
}

int speedToHardwareVoltage = int(map(abs(speed), 0.0F, 1.0f, 0, 255));
Copy link
Member

Choose a reason for hiding this comment

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

redeclaration of variable

@@ -0,0 +1 @@
{"url": "https://docs.google.com/open?id=1HJE8ehG1aPLJJmVXLLJQiW9jOpnDmnVsD7Ys3VNcbWM", "doc_id": "1HJE8ehG1aPLJJmVXLLJQiW9jOpnDmnVsD7Ys3VNcbWM", "email": "[email protected]", "resource_id": "document:1HJE8ehG1aPLJJmVXLLJQiW9jOpnDmnVsD7Ys3VNcbWM"}
Copy link
Member

Choose a reason for hiding this comment

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

This file should not be source controlled

MotorPair.cpp Outdated

MotorPair::~MotorPair()
{
delete left, right;
Copy link
Member

Choose a reason for hiding this comment

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

this does not do what you think it does;
it is the equivalent of

delete left;
right; // this has no effect

right is not deleted

MotorPair.h Outdated

class MotorPair
{
motor *left, *right;
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to just use regular objects instead of pointers.
A pointer here doesn't gain us anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants