-
Notifications
You must be signed in to change notification settings - Fork 641
Convert can.Logger
and can.LogReader
into functions
#1703
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
Conversation
ae5d6dd
to
daf8602
Compare
can/io/logger.py
Outdated
def _get_logger_for_suffix(suffix: str) -> Type[MessageWriter]: | ||
try: | ||
logger_type = MESSAGE_WRITERS[suffix] | ||
if logger_type is None: |
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.
How can a None value make its way into the dictionary? Is there a root cause that would help the user mitigate the problem?
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 was already there since the mf4-PR, but you're right, it's not necessary. I removed it.
can/io/logger.py
Outdated
""" | ||
Logs CAN messages to a file. | ||
real_suffix = pathlib.Path(filename).suffixes[-2].lower() |
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 will throw an IndexError if the filename does not have the necessary number of suffixes. Might be better to catch the case and raise an appropriate message.
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 added a check for the the length of suffixes
The front page of python-can in github gives this code in the Example usage section:
so we might have a lot of people building their code on top of the idea of the |
It still works the same way as before. |
9aea2e0
to
4727ea6
Compare
can.Logger
andcan.LogReader
into functionslisteners.rst
intonotifier.rst
andfile_io.rst
.