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

Library refactor #3

Merged
merged 11 commits into from Jan 30, 2021
Merged

Library refactor #3

merged 11 commits into from Jan 30, 2021

Conversation

joeFischetti
Copy link
Collaborator

Massive changes.
Suggestion would be to verify functionality first. And perhaps branch current master as a version so it can be checkpointed.

Overview:
Individual scripts for checksum, encryption, and compression have been moved into lib directory as libraries that can be imported.
VW_Flash.py is a single point of entry into those operations.

Copy link
Owner

@bri3d bri3d left a comment

Choose a reason for hiding this comment

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

Thank you so much. Looks good for now!.

Long term we should either inject the logger or figure out the "Pythonic" way to do it, to avoid a global. There are some object/binary files in lzss that I'll clean up for ya. And, I guess the use of lib is sort of non-Pythonic as well although I don't mind it.

I'd also, of course, eventually like to factor these file-based utilities into "true" library functions - instead of a "main" , a function that takes the binary input and output, to delegate the file handling to the consumer. In this way we'll eventually be able to build a consumer/UI that doesn't require intermediate files and can do all the processing in memory.

Regardless, none of these are blocking and this is better than where we're at! I will push one more commit to this branch to remove the binaries and then merge this PR, thank you!


rootLogger = logging.getLogger()

def main(inputfile = None, outputfile = None, loglevel=logging.INFO):
Copy link
Owner

Choose a reason for hiding this comment

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

Long term we might as well inject the logger with a default rootLogger = logging.getLogger() here, for example, to avoid the global. Or find whatever the more Pythonic way to do this is. Either way, not blocking 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.

None yet

2 participants