-
Notifications
You must be signed in to change notification settings - Fork 27
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
Extract scripts from non-uflash'd hex files #16
Open
L0C4RD
wants to merge
9
commits into
ntoll:master
Choose a base branch
from
L0C4RD:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
34c331f
Expanded extract_script to work with non-uflash'd hex files.
L0C4RD 8e4be9a
Added me to list of authors (think that's probably fair?)
L0C4RD 898320b
Added the IntelHex module to requirements.txt
L0C4RD 7ec0595
Added real name.
L0C4RD bb0d86b
Added a test for the expanded extract_script(), and made it more robust.
L0C4RD d468c3b
Removed third-party modules; resolved 2.7<->3 compatibility issues
L0C4RD 8741185
Added more 2.7<->3 compatibility fixes; resolved all but three tests.
L0C4RD dc2a007
Un-broke the test_extract_not_valid_hex() function
L0C4RD 8178cbb
Removed IntelHex and StringIO completely.
L0C4RD File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
Nicholas H.Tollervey ([email protected]) | ||
Matt Wheeler ([email protected]) | ||
|
||
Edmond Locard ([email protected]) | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cute. If, by chance, you really are named after the famous French forensics expert please link me to some sort of online presence I can use to verify who you are. Otherwise, pseudonyms are no more helpful than an anonymous gmail username, even for people who probably work in some sort of infosec capacity. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LinkedIn: linkedin.com
Twitter: twitter.com
Talk I did recently (think my name is on one of the slides): youtu.be
I hope it's understandable that I try to minimise my online presence - the work that I do is quite often misrepresented in the media, and on more than one occasion I've had people make threats to me, both across the internet and in person. Unfortunately, having a weird name makes it that much easier to find me.
Because of all this, I don't get as many opportunities to work on amazing community projects as I'd like. However after reading your contributions guidelines here, and being really impressed by what seemed to be a radically inclusive outlook on contributions, I thought you guys might be willing to accept some of my work, even lacking, as I am, a huge public presence - please prove me right!
pokes (as it's been a little while) : @ntoll
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @L0C4RD and many thanks for your patience.
I completely understand and totally sympathise with your reasons for a low profile. I hope you, in turn, can see why I may also have been a tad paranoid about accepting contributions for code relating to a project aimed at kids from an anonymous third party.
Given the links you provided above I'm completely happy to accept your patch and any further contributions you may make!
I really enjoyed your presentation. It had me grinning like a Cheshire Cat. I have a few questions for you about the talk and I wonder if you might contact me via ntoll ntoll org.
Also, you're right about our intentions regarding contributions: radical inclusion is one way to say we want to work in a community that reflects all of us. Diversity is a strength!
Finally, there's a few things I want to fix in the code you've submitted. Did you
make check
..? ;-) There's a bunch of code style warnings that I've fixed locally on my machine. Also, there are some Python 2/3 compatibility issues (StringIO
etc...).Also, you've introduced a dependency on the third party
IntelHex
module. Does this need to be used? One of the "features" of uflash is that it's stand-alone so you could just drop the file into any other project (like Mu) and "it should just work" (tm).Thoughts..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed your email address in the AUTHORS file. I'll drop you a line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha - I did not run
make check
! Apologies, I'll do that on my end and try to get those warnings cleaned up for you, along with those compatibility problems.The
IntelHex
module was just a shortcut for quickly interpreting Intel Hex files, but everything it accomplishes can be achieved with a dict and some parsing code. I'll implement this and pop it into the next commit, so it shouldn't jeopardise uflash's standalone-ness. 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff and many thanks. Looking forward to the revised PR. :-)