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

Cleanup PR to emv contactless to contact bridge #2726

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

n-hutton
Copy link
Contributor

The previous pr #2652 could do with some cleanup - this is one step towards that.

Lots more comments, separates things out a lot more cleanly, removes dead code, and so on.

working demo

works

seems to work so far

more cleanup and works

working copy

working, clean one more pass

cleanup continues

back in buisness babyyy

final cleanup before PR I hope
Copy link

You are welcome to add an entry to the CHANGELOG.md as well

@@ -42,5 +43,6 @@
#define AUTHKEYNONE 0xff

void Mifare1ksim(uint16_t flags, uint8_t exitAfterNReads, uint8_t *uid, uint16_t atqa, uint8_t sak);
bool MifareSimInit(uint16_t flags, uint8_t *uid, uint16_t atqa, uint8_t sak, tag_response_info_t **responses, uint32_t *cuid, uint8_t *uid_len, uint8_t **rats, uint8_t *rats_len);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to avoid code replication, we use the mifare sim init code for evmsim - this means the flags are also set differently in cmdemv.c

@@ -670,7 +670,7 @@ static int CmdEMVSmartToNFC(const char *Cmd) {

CLIParserFree(ctx);

// todo: check this is relevant for us.
// todo for PR: check this is relevant for us.
SetAPDULogging(show_apdu);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

what does SetAPDULogging do?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it shows the tx/rx of apdu messages

@@ -490,6 +490,15 @@ ISO 7816-4 Basic interindustry commands. For command APDU's.
#define ISO7816_GET_CHALLENGE 0x84
#define ISO7816_MANAGE_CHANNEL 0x70

#define ISO7816_APPLICATION_BLOCK 0x1E
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these appeared to be missing..?

Copy link
Collaborator

Choose a reason for hiding this comment

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

good catch

@iceman1001
Copy link
Collaborator

Let me know when you think its ready to merge

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