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

Fixes #84 #89

Merged
merged 2 commits into from
Mar 27, 2024
Merged

Fixes #84 #89

merged 2 commits into from
Mar 27, 2024

Conversation

Juan-789
Copy link
Contributor

When faced with a known but unimplemented subtype, the program previously raised a DataBlockUnknownException in v1/DataBlock, causing disruptions. Additionally, functions like parse_radio_block in telemetry_utils, dependent on this functionality, failed to raise NotImplemented as intended, leading to further issues. This enhancement ensures a more precise exception handling, resolving both the accuracy of the raise statement and the dependency issues encountered by related functions. I have tested all the subtypes and works for everything except for 00 (debug message) which gives ValueError though I have noticed that it returns this even before i put my hand in so ig everything is fixed

…n encountered with subtype that are know yet not implemented
@EliasJRH
Copy link
Contributor

Nice fix! Since we're adding this distinction, we should probably add some error handling in telemetry_utils.py to catch DataBlockUnknownExceptions as well.

Also, some github pr conventions that are good to know, your pull request titles should just be a quick blurb about what the PR changes. If you want to auto close issues using "fixes", that should go in the description of the pull request.

@linguini1
Copy link
Collaborator

I'm not a huge fan of having a dedicated list for not-implemented data blocks, since this will (hopefully) be resolved in the near future as we are implementing them.

In my opinion, I think we can remove the non-implemented list and just try to grab from the subtype dictionary. If subtype comes back as none, instead of raising a UnknownDataBlock error, we would raise the not implemented error. This is because the parameter type does say DataBlockSubtype, so we can assume that only data block subtypes which exist are being passed.

@linguini1 linguini1 merged commit 5385572 into CarletonURocketry:main Mar 27, 2024
4 checks passed
@linguini1
Copy link
Collaborator

@Juan-789 Are you able to create branches/did I add you to the CarletonURocketry organization? If not lmk, I'll add you so that you can create branches instead of forking

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