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

fix(api): account for old plate reader serial format when parsing the device info. #16824

Open
wants to merge 1 commit into
base: chore_release-8.2.0
Choose a base branch
from

Conversation

vegano1
Copy link
Contributor

@vegano1 vegano1 commented Nov 14, 2024

Overview

The RMA plate reader we just got back from byonoy has the old serial format in their eeprom, so we can't parse the serial number correctly, leading to issues addressing the device. Let's change the serial parsing to account for this, so we can continue to use the old byonoy devices.

Test Plan and Hands on Testing

  • Make sure new and old format is parsed correctly
  • Make sure you can update the fw on plate readers with new/old formatted serial
  • Make sure the parsing works with the latest PVT plate readers

Changelog

  • use match instead of fullmatch when parsing the plate reader serial number to account for old format

Review requests

Risk assessment

low, unreleased

@vegano1 vegano1 requested a review from a team as a code owner November 14, 2024 17:29
@vegano1 vegano1 requested review from CaseyBatten, SyntaxColoring and sfoster1 and removed request for a team November 14, 2024 17:29
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

What exactly is the old format that the fullmatch doesn't match? Just extra whitespace around the serial number or something?

Because if it's actual "content," like XYZBYOABC123 instead of BYOABC123, that seems like something that we should capture in the regex instead of discarding it.

@vegano1
Copy link
Contributor Author

vegano1 commented Nov 14, 2024

What exactly is the old format that the fullmatch doesn't match? Just extra whitespace around the serial number or something?

unfortunely its more annoying than that

New format: BYOMAA00013
Old format: BYOMAA00040 REF: DE MAA 001

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Haha oof. Okay, yeah, this makes sense. Though, can we either add:

  • Tests covering all the formats that we're trying to handle
  • Or a comment next to SERIAL_PARSER giving examples of all the formats that we're trying to handle

@vegano1
Copy link
Contributor Author

vegano1 commented Nov 14, 2024

Never mind, there's a deeper issue that I'll need to look into.
The version format is also different now, ill re-open this pr when ready.

@vegano1 vegano1 closed this Nov 14, 2024
@vegano1 vegano1 reopened this Nov 14, 2024
@vegano1
Copy link
Contributor Author

vegano1 commented Nov 14, 2024

I'm re-opening this to at least fix the serial number parsing.
There will be a separate pr to fix the versioning once we get word from the OEM.

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