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 rev6-7 support #107

Merged
merged 3 commits into from Nov 19, 2017
Merged

add rev6-7 support #107

merged 3 commits into from Nov 19, 2017

Conversation

zoogie
Copy link
Contributor

@zoogie zoogie commented Nov 16, 2017

And maybe rev8 but I haven't seen anybody with this card yet.

We still need more command C7 samples from users to safely determine r4ids.cn revisions.
@d3m3vilurr knows what I mean by this. Might need to open some rev4-5 issues and ask users as I did here: #9 (comment)

But this did work for me on my 3 r4igolds and at least two other users on rev6 and 7. This should complete full support for the r4ids.cn : )

We still need more command C7 samples from users to safely determine r4ids.cn revisions.
@d3m3vilurr
Copy link
Contributor

sorry @zoogie. I attached my experimental changes.

@zoogie
Copy link
Contributor Author

zoogie commented Nov 16, 2017

rev4-5 is broken now.

This is wrong
blowfish = encrypt
firm_payload = encrypt
firm_header = encrypt

This is right
blowfish =plaintext
firm_payload = encrypt
firm_header = plaintext

1. fix whitespaces
2. merge all injection methods;
  these methods just have little diffs(mostly offsets).
  so added setting structure, then merge all logics

  side effect:
    - we need multiple read firm for the flash bf and firm header
    - got warnings about designated initializers. I'm too lazy

3. extract injectFlash method; read~inject data~write chain
@d3m3vilurr
Copy link
Contributor

@zoogie yeah right. fixed my mistake.

@zoogie
Copy link
Contributor Author

zoogie commented Nov 17, 2017

Alright, everything works now. LGTM
Sorry about the whitespace issues, don't know what causes that. Next time I'll make sure it's cleaned up.

@kitlith
Copy link
Member

kitlith commented Nov 17, 2017

@zoogie there's no need to go and explain specifically to me in the issues where you're asking these questions, it's fine. 😛

Might want to consider adding an enum to select the cart type, and also I think we're sticking with a relative include when including device.h, so #include "../device.h" was correct. </nitpick>

LGTM!

@angelsl
Copy link
Member

angelsl commented Nov 18, 2017

Best to stick with an include directive that is satisfied without assuming anything about the include paths defined by the application using us.

i.e. #include "../device.h"

Don't rely on the environment setting a directory as an include directory.
@kitlith kitlith merged commit 77f86c5 into ntrteam:master Nov 19, 2017
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

5 participants