forked from micropython/micropython
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Implement hashlib.new('sha256') and enable it for CLUE #10740
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
3453683
implement hashlib.new('sha256') support
samblenny f80d992
enable hashlib sha1 & sha256 for CLUE board
samblenny 30c8dd6
default to enabling SHA256 when hashlib is enabled
samblenny f9d5778
turn on hashlib for all nrf52840 boards
samblenny fabe72b
remove redundant hashlib enable for CLUE
samblenny c1bd53d
remove CIRCUITPY_HASHLIB_SHA256 ifdef guards
samblenny 39332eb
fix hashlib.new() documentation comment
samblenny 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 hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
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.
Need this still? Looks like it fit everywhere.
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.
I agree maybe just take this out and take out the guards
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.
That's the bit that lets me actually use it on the board I'm implementing this for as the nordic port doesn't turn hashlib on by default. Are you suggesting maybe do a CIRCUITPY_HASHLIB = 1 at the port level for nordic?
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.
Ah, ok. This is fine then.
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.
Ah, maybe @dhalbert is saying to remove CIRCUITPY_HASHLIB_SHA256 which you also don't need. So, leave CIRCUITPY_HASHLIB here but remove the new _SHA256 define.
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.
Looking at ports/nordic/mpconfigport.mk, I see a comment suggesting that space is tight on nrf52833. What if I just enable hashlib in the
ifeq ($(MCU_CHIP),nrf52840)section for the nrf52840 boards?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.
I just pushed a couple commits to enable hashlib at the port level for nrf52840 boards (not nrf52833 though) and remove the redundant hashlib enable for CLUE.
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.
This is one of the things I was saying. If hashlib is enabled, don't do finegrain choices on the algorithms right now. I think if there's room for SHA1, there'd be room for SHA256.
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.
so, should I remove the ifdef guard for CIRCUITPY_HASHLIB_SHA256?
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.
yes, I think you can just get rid of it, if it fits on the Espressif and Pico W.