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

ACR125 has the same LED/Buzzer support as ACR122 :) #111

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

JohnMcLear
Copy link
Contributor

No description provided.

@pokusew
Copy link
Owner

pokusew commented Feb 18, 2021

Hi @JohnMcLear,

Thank you for your PR. 👍

I cannot find any ACR125 (Didn't you mean ACR1252U?). Could you please send link to the specific ACS product / docs page (so we can add it as comment into the code)? Then, I'll merge the PR.

BTW: It would be much cleaner to create a new subclass and extract the common functionality (e.g. the buzzer command) into some shared helper function But for now, I am more than okay with your patch. 🙂

@JohnMcLear
Copy link
Contributor Author

Yes ACR1252U https://www.acs.com.hk/en/products/342/acr1252u-usb-nfc-reader-iii-nfc-forum-certified-reader/

Sorry RE code quality, noted RE future approaches, and yeah, agreed.

@pokusew pokusew merged commit 181cf88 into pokusew:master Feb 18, 2021
@pokusew
Copy link
Owner

pokusew commented Feb 18, 2021

@JohnMcLear Thanks. 👍 Shipped in v0.8.1. 🚀

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.

None yet

2 participants