Skip to content

Conversation

@bettse
Copy link
Collaborator

@bettse bettse commented Jan 14, 2024

What's new

  • Better, more picopass specific, UI during dictionary
  • Use elite dictionary attack for default read since our system dictionary has 700 keys and otherwise the lack of screen update is confusing

Screenshot-20240113-224344

Verification

  • Try reading a card, see progress bar

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

@bettse
Copy link
Collaborator Author

bettse commented Jan 14, 2024

@nvx / @pcunning any thoughts/concerns?

@nvx
Copy link
Contributor

nvx commented Jan 15, 2024

sounds like a good change to me, just to confirm it still does standard keys as well first not just elite?

likely won't get a chance to actually test the code for a few days at least

@bettse
Copy link
Collaborator Author

bettse commented Jan 15, 2024

sounds like a good change to me, just to confirm it still does standard keys as well first not just elite?

💯 . it does system standard, user elite, system elite

@bettse
Copy link
Collaborator Author

bettse commented Jan 21, 2024

@nvx let me know when you've had a chance to try this (if you want to) and then I'll mark it ready for review

@nvx
Copy link
Contributor

nvx commented Jan 21, 2024

@nvx let me know when you've had a chance to try this (if you want to) and then I'll mark it ready for review

I completely forgot about this, I've had a brief look and seems fine so I reckon send it.

@bettse bettse marked this pull request as ready for review January 21, 2024 23:14
@skotopes
Copy link
Contributor

@bettse I've approved PR. Feel free to merge it.

However there are couple things that bothers me a little bit:

  • Leftovers from read scene
  • Elite read scene doesn't have apply card to the back at the beginning.

@bettse
Copy link
Collaborator Author

bettse commented Jan 23, 2024

leaving the read code in for the moment is intentional: I figured if we needed to revert it make the diff simpler/smaller and easier. I plan to clean it up if we see this working well and don't hear of any problems.

I'll look into the apply card to the back and see if I can introduce that.

@bettse
Copy link
Collaborator Author

bettse commented Jan 23, 2024

I checked and neither had a literal apply card to the back, Read just says "detecting picopass". I think I'm going to move forward with this, but make a note to work on the flow. I think it should have some sort of apply card to the back at first, and I also think the elite dictionary handling of the tag being removed is less than ideal (seems to treat it like 'skip')

@bettse bettse merged commit a95c9f9 into flipperdevices:dev Jan 24, 2024
@bettse bettse deleted the picopass_read_improvements branch January 24, 2024 02:36
@bettse bettse mentioned this pull request Jan 27, 2024
3 tasks
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