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

Bugfix: re-enable Mifare Ultralight C support #2214

Merged
merged 8 commits into from
Jan 30, 2024

Conversation

miohna
Copy link
Contributor

@miohna miohna commented Jan 11, 2024

Make use of read_id function from pirc522 package to re-enable Mifare Ultralight C support.

Since the change of the pirc522 repo, cf. #2066, #2075, #2078, I have realized issues with Mifare Ultralight C stickers in my setup (RPi Zero 2). Moving from the previous "more manual" check of uid to the read_id function of the pirc522 package fixes this issue.

ATTENTION:
Previously, a non-standard representation of NFC card uid was returned by the Mfrc522Reader class. With this change a standard representation of the NFCs uid is return which breaks all currently stored links between cards and actions. Writing a converter appears not possible due to the way "uids" were put together previously. Like in future3 the old variant was labelled "legacy".

Make use of read_id function from pirc552 package to reenable Mifare
Ultralight C support.

ATTENTION:
Previously a non-standard representation of NFC card uid was returned by
the Mfrc522Reader class. With this change a standard representation of
the NFCs uid is return which breaks all currently stored links between
cards and actions.
@coveralls
Copy link

coveralls commented Jan 11, 2024

Pull Request Test Coverage Report for Build 7705488041

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 76.82%

Totals Coverage Status
Change from base Build 7458174278: 0.0%
Covered Lines: 633
Relevant Lines: 824

💛 - Coveralls

@s-martin
Copy link
Collaborator

If I understand you correctly, this will be a breaking change on existing installations.

future3 asks, if the „legacy“ mode shall be used during installation.

We need to check, how this could be implemented with little impact.

@s-martin s-martin added bug investigation needed legacy_v2 Issues, discussions and PRs related to Version 2.x labels Jan 11, 2024
Add a legacy mode similar to 'future3' implementation.
Use the legacy mode to allow correction of card uid read with Mfrc522
while keeping the class backward compatibil, cf. 'future3' handling.
@miohna
Copy link
Contributor Author

miohna commented Jan 17, 2024

Like in future3, I have now implemented a „legacy“ mode that can be deactivated via a file in the settings folder (Mode_Legacy).
By default the well known behavior of the Mfrc522Reader is active. Therefore, the chance to the Mfrc522Reader Class will not break any existing installations anymore.

@s-martin Thanks for pointing out this issue.

scripts/Reader.py.experimental Outdated Show resolved Hide resolved
scripts/Reader.py.experimental Outdated Show resolved Hide resolved
@AlvinSchiller
Copy link
Collaborator

AlvinSchiller commented Jan 18, 2024

Maybe also add this to the RC522 setup, so the user can choose at this point which mode to use?

@s-martin s-martin added this to the v2.6 milestone Jan 23, 2024
Modified general legacy mode to a specific option of the rc522 reader
that allows to change between the read of the usual UID and the read of
the previous custom implementation of card ID.
Extend rc522 setup script to question user for the usage of UID mode
rather than the old custom card ID.
@miohna
Copy link
Contributor Author

miohna commented Jan 24, 2024

Maybe also add this to the RC522 setup, so the user can choose at this point which mode to use?

See my attempt in cd883df.
question function was not general enough to be used. To keep changes minimal I have reduced the case options for the UID question to yes and others.

@AlvinSchiller
Copy link
Collaborator

Looks good to me now :)
The ids i tested are the same as in v3 so works as expected

@s-martin s-martin merged commit 01734de into MiczFlor:develop Jan 30, 2024
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug legacy_v2 Issues, discussions and PRs related to Version 2.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants