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 support for Accu-Chek meters and pumps using Smart Pix (UPLOAD-685, UPLOAD-872) #1546

Merged
merged 6 commits into from
Apr 4, 2024

Conversation

gniezen
Copy link
Member

@gniezen gniezen commented Feb 20, 2023

This PR continues the excellent work done by @mrinnetmaki and @jlaunonen in #1432 to add support for the Roche Accu-Chek Smart Pix device. It handles both meters and the Spirit/Combo pump.

I created a new PR as our CI/CD pipeline will not build binaries for external PRs, and I don't have permission to push to external PRs either.

@gniezen gniezen marked this pull request as ready for review February 20, 2023 14:38
@gniezen gniezen requested a review from krystophv February 20, 2023 14:38
Copy link
Member

@krystophv krystophv left a comment

Choose a reason for hiding this comment

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

While stylistically fairly different than most of the codebase, this seems largely sound. I have a few questions/comments that I'd like to get a bit of feedback on before giving it the thumbs up.

lib/drivers/roche/accuChekSmartPix.js Outdated Show resolved Hide resolved
lib/drivers/roche/smartpix/accuCheckSmartPixMeterXML.js Outdated Show resolved Hide resolved
lib/drivers/roche/accuChekSmartPix.js Outdated Show resolved Hide resolved
lib/drivers/roche/smartpix/accuChekSmartPixPumpXML.js Outdated Show resolved Hide resolved
krystophv
krystophv previously approved these changes Feb 23, 2023
Copy link
Member

@krystophv krystophv left a comment

Choose a reason for hiding this comment

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

Post review updates LGTM 👍🏼

@mrinnetmaki
Copy link
Contributor

mrinnetmaki commented Jul 17, 2023

The VCLA should be in order, see #1432 (comment).

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