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

Pq to class #404

Merged
merged 7 commits into from
Jun 5, 2024
Merged

Pq to class #404

merged 7 commits into from
Jun 5, 2024

Conversation

tomyyyD
Copy link

@tomyyyD tomyyyD commented May 29, 2024

Update the priority queue to be a class. Simplifies creating new queues later for image transmission. Closes #390.

Copy link

@akollgaa akollgaa left a comment

Choose a reason for hiding this comment

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

All the code is clear. Although this is not needed, I think the usage message at the top of PriortyQueue should be rewritten. It is a little misleading because it alludes to the old usage of priority_queue.py.

def __str__(self) -> str:
s = '['
for idx, item in enumerate(self.queue):
s += f'(idx: {idx}, priority: {item.priority}, {item.contents})'

Choose a reason for hiding this comment

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

Currently self.queue only seems to contain Message objects from radio_utils/message.py. The message class does not have a .contents attribute. I just wanted to point this out because you didn't seem to allude to this at all unless it is your intention to have this used for image_queue.

Copy link
Author

Choose a reason for hiding this comment

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

good catch. We can just print out the object identifier instead of the contents

>>> heap.push(item) # pushes a new item on the heap if under limit
>>> item = heap.pop() # pops the largest item from the heap
>>> item = heap.peek() # largest item on the heap without popping it
>>> heap.heapifu() # transforms list into a heap, in-place, in linear time

Choose a reason for hiding this comment

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

Spelling mistake on line 12: heapifu() should be heapify()

@tomyyyD tomyyyD merged commit dde0e59 into main Jun 5, 2024
5 checks passed
@tomyyyD tomyyyD deleted the pq_to_class branch June 5, 2024 13:30
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.

Cleanup image/transmission queues to be instances of a priority queue class
3 participants