-
Notifications
You must be signed in to change notification settings - Fork 51
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
Tomer fixes #110
Tomer fixes #110
Conversation
8a0b474
to
106daae
Compare
Current code prevents negative for min_payload_len_px but not for max_payload_len_px
We throw a lot of exceptions in our USB detection code. This commit gathers all these error cases into throwing a single DymoUSBError. Now in case of any error, both command line and GUI application will print a single line, unless DEBUG or VERBOSE environment variables are set, in which a traceback will be printed. A common crash warning message box that shows traceback is also added.
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 looks really amazing as usual @tomers!!! These are some really huge improvements to the quality and structure of the code.
I think there's a design aspect that we should ensure we agree on: the debug/verbose mode. Given the experimental nature of this project, I feel inclined to make things verbose by default. If a user asks for help it's just another step to ask them to turn on debug to get the stack trace. If we do decide it's the way to go, I would have expected to see it via command line arguments, and not just through environment variables. What do you think?
I think the program needs to be regarded as stable with regards to how it looks, i.e. there should not be traceback information shown in dialog boxes, etc. Our users are not software developers, so they should not see it. |
Show label width beneath the render label. It is a common use case that user would want to know the real width of the printed label.
Always show print button (which can be disabled). The optional error label is beneath the print icon.
Used to catch any sort of exception, and print it properly
Always show print button (which can be disabled). The optional error label is beneath the print icon.
Now that all logging is not shown by default, due to use of Python standard logger, we would like to print the errors to console.
When connecting a device, there's a short phase in which this error is shown. Since it is very long, and is shown in the error widget, the whole app's layout is changed. Instead, only show abbreviated message in the error label, and dump instructions to the console (we might want to have a dialog box for that in the GUI application in the future).
Coding convention in this project is up to 88 chars per line
Python's naming convention for function is to use a lowercase word or words. Separate words by underscores to improve readability.
Filename should match name of main class, DymoLabeler
Have separate class for each kind of rendered (text, barcode, QR, etc.). Combine renders are done using another kind of render (HorizontallyCombinedRenderEngine). Move logic to associate renders and dymo device in a separate labeler_device file. Note that the new architecture is design to support multiple labelers in the future. This commit is the first step in that direction.
Prevents USB device polling from emitting the same error message over and over again.
Amazing work as usual, thanks so much @tomers!!!!! |
Some more fixes :-)