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

add methods for circular buffer #279

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

Nicolasara
Copy link

No description provided.

@Nicolasara Nicolasara requested a review from a team as a code owner October 5, 2023 02:02
@Nicolasara Nicolasara linked an issue Oct 5, 2023 that may be closed by this pull request
@SamHaggans
Copy link
Contributor

This is great work so far! I do think, though, that these added methods would fit better in a new class that uses either inheritance or composition to use the base CircularBuffer code but then exposes new functionality in a new class.

@Nicolasara
Copy link
Author

Sam explained to me that this is used to average the values of some of the sensors while it was in the launch pad, I was thinking it could be useful to have a standard deviation method that calculates what the standard deviation is of all the values. I am guessing that if a sensor has a high standard deviation it might mean its not working well and is being less accurate, although not sure how helpful this would be, still new to all this avionics stuff.

@Nicolasara
Copy link
Author

The cmake checks that are not passing are the ones you mentioned right @SamHaggans?

@mcm001
Copy link
Contributor

mcm001 commented Oct 12, 2023

Sam explained to me that this is used to average the values of some of the sensors while it was in the launch pad, I was thinking it could be useful to have a standard deviation method that calculates what the standard deviation is of all the values. I am guessing that if a sensor has a high standard deviation it might mean its not working well and is being less accurate, although not sure how helpful this would be, still new to all this avionics stuff.

More statistics could be good! Would be cool to plot that over time. That, and sensors can sometimes fail "railed" where they get stuck at their last value or end up stuck at their upper or lower limits. Would be cool to see that plotted over time on some old flight data maybe

@mcm001
Copy link
Contributor

mcm001 commented Oct 12, 2023

The cmake checks that are not passing are the ones you mentioned right @SamHaggans?

Yep, the windows cmake ones? That's not something you broke


template <typename Type, size_t Capacity>
class CircularBufferUtility {
CircularBuffer<Type, Capacity>* cb;
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this is a pointer type (rather than the "full" circular buffer being a member variable), we'd need to allocate the buffer itself somewhere else and then give an object of this class a pointer to that buffer. I think it would be a lot easier to use if this class had a full circular buffer and not a pointer so that we avoid that issue.

Also, look into adding access modifiers public and private here to make the circular buffer member private and the rest of the methods public (see the original circular buffer class as an example).

* @return bool: Status, true if dequeue and enqueue succeeds, false if either
* fails.
*/
* / bool addElement(Type item) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a trailing comment block got added here

Type sum() {
Type sum = 0;
for (size_t i = 0; i < this->cb->count(); i++) {
Type item = this->getNthValue(i);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be updated to use the new getNthValue that does it through a pointer.

return false;
}

if (this->head + n <= endPtr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this class isn't extending the circular buffer class and is just using composition, it doesn't have the head and tail and backingarray of the circular buffer. They're also private inside the circular buffer, so they couldn't be accessed here through it, either (i.e. this->cb->head will not work because the head is private).

I think that means that this function probably needs to go into the actual circular buffer class and then this class can expose the same function that delegates to the circular buffer.

@SamHaggans
Copy link
Contributor

#79

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.

Generalized class to replace running buffers used in filters
3 participants