My code implementation for software challenge #3
Conversation
There was a problem hiding this comment.
Hi Brendon, great work on this! I really like the way that you implemented looping through the three different values in the tuples, it is a nifty way of doing it. Just a couple of general comments, and one question for you. Go ahead and address and then I can take another look!
| if self.current_position == self.desired_position: | ||
| print("Current position is already at desired position.") | ||
| return True |
There was a problem hiding this comment.
Good to deal with this case first, well done!
fall-2020/navigation.py
Outdated
| if coordinate_index == 0: | ||
| print("Unexpected input for x coordinate. Should be behind target not in front of target.") |
There was a problem hiding this comment.
Question for you - how do you think we should deal with this? I notice that you do not return false here, maybe we should be treating this as an error
fall-2020/navigation.py
Outdated
| if self.current_position == self.desired_position: | ||
| if self.steering.stop(): | ||
| print("Stop successful. All actuation's successful.") | ||
| return True | ||
|
|
||
| else: | ||
| print("All actuation's successful but failed to stop.") | ||
| return False | ||
|
|
||
| else: | ||
| if self.steering.stop(): | ||
| print("Actuation's unsuccessful desired destination not reached.") | ||
| return False | ||
|
|
||
| else: | ||
| print("Actuation's unsuccessful and failed to stop.") | ||
| return False |
There was a problem hiding this comment.
This is a nice logical sequence of the possible outcomes of the code, makes it very easy to understand what is going on. Great work here!
| # Looping through and comparing each corresponding coordinate in the two position tuples. | ||
| for coordinate_index in range(3): | ||
| # If x => check if TBM is behind, if y => check if TBM is left, if z => check if TBM is below. |
There was a problem hiding this comment.
I think your comments here really do help explain exactly what is happening in the code. Great work!
There was a problem hiding this comment.
Daniel covered a lot of the logical things, so I'll focus on the code and submission style:
Git Practice / Submission Style:
- Branch Name - As requested, nice!
- PR Title - Doesn't match the requested format, however we did add this request late and you may have read the readme before that was added - that's on us!
- PR Description - Although descriptive, the description is very hard to read, and at a glance, it is very hard to discern any information from it. It is good to shoot for a brief summary, potentially with some bullet points describing the changes as concisely as possible.
- Commit Messages - Remember to include a short, one sentence-ish description of what you added in your commit messages.
Code Style:
- Overall, the code style is great! There are a few comments pertaining to unnecessary blank lines, but its all really minor style stuff.
Thanks for the submission!
fall-2020/navigation.py
Outdated
| elif coordinate_index == 1: | ||
| if self.steering.move_right(steer_distance): | ||
| print("Actuation successful.") | ||
|
|
There was a problem hiding this comment.
Don't think you need to put a blank line between the if and else statement blocks - also applies to all other if else's you have in this submission. It is a little more justified for the outer level of if/else, however for the nested ones I think it would be cleaner to not have a blank line.
fall-2020/navigation.py
Outdated
|
|
||
|
|
There was a problem hiding this comment.
python methods should be separated by a single line blank line.
Added my implementation of the navigation class. Used the pollSensor method from the GPS class to get the current position of the TBM. Within the navigation class the update_current_position method is used to find the current location of the TBM. Then the current position is checked with the desired position and if the TBM is already at the desired position the method returns true. If the TBM is not at the desired location, then a for loop is implemented to loop through and compare the x, y, and z coordinates from both position tuples. Based on the comparison, the steer distance is calculated and the necessary method for steering is called from the Steering class. With each steering actuation, the code checks if the actuation is successful, and if it is not successful a message is printed out with what actuation failed. After all actuations are completed then the update_current_position method is called again to get the new current position of the TBM. If the new current position of the TBM matches the desired position then all previous actuations were successful, and the method returns true. However, if the stop method fails to execute then the method returns false, indicating that all previous actuations were successful but the stop actuations was not successful.