Ackermann steering #128
Ackermann steering #128
Conversation
| return hal::success(); // Is there a hal::failure()? | ||
| } | ||
|
|
||
| for(size_t i = 0; i < m_legs.size(); i ++) { |
There was a problem hiding this comment.
a better way to iterate over a span could be a range base loop like so:
for (auto& value : values)
There was a problem hiding this comment.
This can't be done using a for each, since I still need the iterator to index the other span.
drive/include/settings.hpp
Outdated
| #include "drive_configuration.hpp" | ||
| namespace sjsu::drive { | ||
| // Multiply this with the desired angle to get the correct angle to steer the motors. | ||
| constexpr float angle_correction_factor = 1.3625; // ???? WHY ARE THESE MOTORS SCALED???? |
drive/include/settings.hpp
Outdated
| #include "drive_configuration.hpp" | ||
| namespace sjsu::drive { | ||
| // Multiply this with the desired angle to get the correct angle to steer the motors. | ||
| constexpr float angle_correction_factor = 1.3625; // ???? WHY ARE THESE MOTORS SCALED???? |
There was a problem hiding this comment.
Where does this number come from? What does it mean, improve documentation and possibly a better name
| * @brief This structure defines a valid steering configuration for the drive system. | ||
| * | ||
| */ | ||
| struct drive_configuration { |
There was a problem hiding this comment.
why not use hal::units. And why are these in deg/sec instead of degrees
drive/include/settings.hpp
Outdated
|
|
||
| // These control how fast the system can react to changes in the target. | ||
| constexpr drive_configuration config_max_delta = { | ||
| .steering_angle = 40, // degrees/sec |
There was a problem hiding this comment.
use units native to what you are trying to compute. Ex: for rmd use rpms and degrees
| * | ||
| * @param dt Change in time since last call | ||
| */ | ||
| void update(float dt); |
There was a problem hiding this comment.
better name for change in time (dt)
| drive_configuration get_current(); | ||
| /** | ||
| * @brief Get the current target | ||
| * | ||
| * @return drive_configuration | ||
| */ | ||
| drive_configuration get_target(); | ||
| /** | ||
| * @brief Get the difference calculated in the last update | ||
| * @note units/sec | ||
| * | ||
| * @return drive_configuration | ||
| */ | ||
| drive_configuration get_delta(); |
There was a problem hiding this comment.
why do these return drive_configuration
| drive_configuration get_current(); | ||
| /** | ||
| * @brief Get the current target | ||
| * | ||
| * @return drive_configuration | ||
| */ | ||
| drive_configuration get_target(); | ||
| /** | ||
| * @brief Get the difference calculated in the last update | ||
| * @note units/sec | ||
| * | ||
| * @return drive_configuration | ||
| */ | ||
| drive_configuration get_delta(); |
There was a problem hiding this comment.
update the documentation for the return type
| #include "ackermann_steering.hpp" | ||
|
|
||
| namespace sjsu::drive { | ||
| /** |
There was a problem hiding this comment.
more detail on what settings is, elaborate on tri wheel configuration, and a brief line to explain purpose of serial.
drive/include/rules_engine.hpp
Outdated
| * | ||
| * @note It is possible to have any steering angle or wheel heading, since the steering function is continous. | ||
| * | ||
| * @param p_config |
There was a problem hiding this comment.
add more explanations for every parameter in this pr
Implemented Ackermann Steering
This is only an change in the steering math for the drive system.
Anything unrelated to the steering was not changed.
Unnecessary files were deleted.
Does not include: