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

Race Tracker #18

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Race Tracker #18

wants to merge 5 commits into from

Conversation

KingSyclone
Copy link

No description provided.

@KingSyclone KingSyclone requested a review from RCMast3r November 6, 2024 00:32

find_package(mcap REQUIRED)

# custom packaged libraries, code didnt require changes just the packaging
find_package(dbcppp CONFIG REQUIRED)
find_package(foxglove_websocket REQUIRED)
find_package(cmake_macros REQUIRED)
find_package(GTest REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

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

please leave GTest in

CMakeLists.txt Outdated
gtest
)

add_test(NAME MyTest COMMAND alpha_test)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is how you add tests, please leave this in

Comment on lines 173 to 178
vn_ins_msg->set_ins_mode(static_cast<hytech_msgs::INSMode>(ins_status & 0b11));
vn_ins_msg->set_gnss_fix((ins_status >> 2) & 0b1);
vn_ins_msg->set_error_imu((ins_status >> 4) & 1);
vn_ins_msg->set_error_mag_pres((ins_status >> 5) & 0b1);
vn_ins_msg->set_error_gnss((ins_status >> 6) & 0b1);
vn_ins_msg->set_gnss_heading_ins((ins_status >> 8) & 0b1);
vn_ins_msg->set_gnss_compass((ins_status >> 9) & 0b1);
Copy link
Contributor

Choose a reason for hiding this comment

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

please leave this in

@@ -60,8 +61,8 @@ namespace core

public:
using tsq = core::common::ThreadSafeDeque<std::shared_ptr<google::protobuf::Message>>;
StateEstimator(core::Logger &shared_logger, std::shared_ptr<loggertype> message_logger, estimation::MatlabMath& matlab_estimator)
: _logger(shared_logger), _message_logger(message_logger), _matlab_estimator(matlab_estimator)
StateEstimator(core::Logger &shared_logger, std::shared_ptr<loggertype> message_logger, estimation::MatlabMath& matlab_estimator, int mode)
Copy link
Contributor

Choose a reason for hiding this comment

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

dont construct the lap tracker in the state estimator, just pass a reference to an already constructed instance.

Comment on lines +22 to +41
private:
bool started;
int lapCount;
int lapNumber;
double startLineLat1, startLineLon1, startLineLat2, startLineLon2;
double finishLineLat1, finishLineLon1, finishLineLat2, finishLineLon2;
double totalDistance;
std::chrono::steady_clock::time_point startTime;

DataPoint previousPoint;
bool hasPreviousPoint;

void calculateStartLine(const DataPoint& point);
void calculateFinishLine(const DataPoint& point);
bool checkLapCompletion(const DataPoint& previousPoint, const DataPoint& currentPoint);
bool checkFinishLineCrossing(const DataPoint& previousPoint, const DataPoint& currentPoint);
double calculateDistance(const DataPoint& point1, const DataPoint& point2);
bool checkLineCrossing(const DataPoint& previousPoint, const DataPoint& currentPoint, double lat1, double lon1, double lat2, double lon2);
DataPoint process_race_data(const core::VehicleState &current_state);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

please follow the naming convention that all member variables and functions have a leading underscore

Comment on lines +34 to +39
void calculateStartLine(const DataPoint& point);
void calculateFinishLine(const DataPoint& point);
bool checkLapCompletion(const DataPoint& previousPoint, const DataPoint& currentPoint);
bool checkFinishLineCrossing(const DataPoint& previousPoint, const DataPoint& currentPoint);
double calculateDistance(const DataPoint& point1, const DataPoint& point2);
bool checkLineCrossing(const DataPoint& previousPoint, const DataPoint& currentPoint, double lat1, double lon1, double lat2, double lon2);
Copy link
Contributor

Choose a reason for hiding this comment

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

please snake_case all functions

point.timestamp = std::chrono::duration_cast<std::chrono::microseconds>(
std::chrono::high_resolution_clock::now().time_since_epoch()).count();
double speed = in_msg->vn_vel_m_s().x();
std::string raceStatus = _lapTracker.processRaceData(point, speed);
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this function call? does not appear to exist.

@@ -27,7 +27,7 @@ namespace common
std::unique_lock lk(_input_deque.mtx);
_running = true;
}
_options.noChunking = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

please keep noChunking= true;

Copy link
Contributor

Choose a reason for hiding this comment

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

also, what about the ability to reset the lap tracker? and / or restart the tracker itself

@@ -122,6 +123,7 @@ namespace common
std::unique_lock lk(_logger_mtx);
msg_to_log.channelId = _msg_name_id_map[msg.message_name]; // under the mutex we also lookup in the map
auto write_res = _writer.write(msg_to_log);
_writer.closeLastChunk();
Copy link
Contributor

Choose a reason for hiding this comment

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

pls remove this

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.

2 participants