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

Feature/circular queue #55

Merged
merged 8 commits into from
Jun 27, 2024
Merged

Feature/circular queue #55

merged 8 commits into from
Jun 27, 2024

Conversation

RRahul2004
Copy link
Contributor

Description

What was completed, changed, or updated?

Implemented TM Cicular Queue


Why was this done (if applicable)?

to allow Tm to store MAVLINK message in queue waiting for use


Testing

What manual tests were used to validate the code?


What unit tests were used to validate the code?


Documentation

Milestone number and name:

Link to Asana task:

Link to Confluence documentation:


Reminders

  • Add reviewers to the PR

  • Mention the PR in the appropriate discord channel

Copy link
Contributor

@HardyYu HardyYu left a comment

Choose a reason for hiding this comment

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

Good try! I listed some points that could cause this class to not work, please review and change them. Don't be afraid of changing someone else's code! Sometimes other people's code is faulted or no long apply to the current use case anymore. I also recommend to include your header in the Boardfiles/nucleol552zeq/Core/Src/main.cpp to check whether your code can pass compilation. The way how compiler works is that if you don't use it, then it won't be compiled. You can do some testing in the main to check whether or not your code is actually working.

TelemetryManager/Src/TMCircularBuffer.cpp Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@HardyYu HardyYu left a comment

Choose a reason for hiding this comment

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

Nice! Yea definitely better than last time. Left some comments to be addressed.

Drivers/common/circular_buffer/src/circular_buffer.cpp Outdated Show resolved Hide resolved
TelemetryManager/Inc/TMCircularBuffer.hpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@HardyYu HardyYu left a comment

Choose a reason for hiding this comment

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

Looks good. Please address the new comments I added and I will give approval to this pr :)

Drivers/common/circular_buffer/inc/circular_buffer.hpp Outdated Show resolved Hide resolved
TelemetryManager/Src/TMCircularBuffer.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@HardyYu HardyYu left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for making the change!

@RRahul2004 RRahul2004 merged commit a152bff into main Jun 27, 2024
2 checks passed
@RRahul2004 RRahul2004 deleted the feature/circular_queue branch June 27, 2024 18:43
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.

3 participants