Refactor callbacks

[rockstorm101]

Here is some more refactoring effort. As always this is up for discussion. In this case I'm suggesting some deprecations so we must have consensus on this. Also thorough testing would be nice.

The ultimate goal is to reduce the size of the printcore class in order to increase modularity and therefore get code that easier to read and maintain. In this PR the idea is simply to:

• Extract the callbacks functionality into a separate class.
• Combine the calls to callbacks and event handlers to optimize code reuse.
• As an extra, a lot of documentation for both callbacks and event handlers is introduced. Both features were not properly documented before.

As of today, the total lines of code is not truly reduced because there is a lot of code kept for backwards compatibility. But, in the future, printcore will have less attributes (all *cb attributes would be removed) and less code lines overall.

$ cloc --by-file --git --diff master refactor-callbacks
File                          comment           code
------------------------------------------------------
printrun/pronterface.py
 same                             185           2142
 modified                           0              8
 added                              0              0
 removed                            0              0
printrun/pronsole.py
 same                             118           1475
 modified                           0              7
 added                              0              0
 removed                            0              0
printrun/printcore.py
 same                             161            383
 modified                           0             40
 added                            119             69
 removed                            4             56
tests/test_printcore.py
 same                             123            512
 modified                           0              1
 added                              2              3
 removed                            0              1
printrun/eventhandler.py
 same                              14             28
 modified                          16              1
 added                              0              0
 removed                           42              0
------------------------------------------------------
SUM:
 same                             601           4540
 modified                          16             57
 added                            121             72
 removed                           46             57
------------------------------------------------------

Regarding the deprecations, the idea would be to release at least one minor version with the warnings in place for users to have time to adapt before releasing a major version which would finally remove the compatibility code.

Regarding the test suite. It now throws tons of warnings. This is intentional to prove two things: a) the same functionality we had before the refactoring is kept and b) the warnings are triggered and are the same as would be seen by users. Once the backwards compatibility code is removed the test suite should be updated.

Why did I change the name of callback/event "preprintsend" to "printpresend"? Because I had to change the signature of the callback function to align it with the event function. Such is a breaking change, so I had to come up with a new name to keep the old name still working until the removal takes place.

If anything else needs explaining/justifying please let me know.

Again, comments/suggestions/criticisms are all welcome.

[neofelis2X]

Hi,
I don't know much about the inner workings of the printcore, so I cannot give very specific feedback. Generally it looks like a great idea and a good improvement to the code. I run some tests on macos and everything works as expected.

• Is there a specific case that should be tested, aside from regular printing?

I had one comment on the documentation of the eventhandler, but thats about it.

[rockstorm101]

I run some tests on macos and everything works as expected.

* Is there a specific case that should be tested, aside from regular printing?

Thanks a lot for testing it! I did not think of any particular test that needs to be done really. Not sure what might break. I guess users that use callbacks and/or addons (if there are any) could be affected the most. Other than that, pronsole and pronterface should (in theory) behave the same.

I'm not quite certain why some of the docstrings have been removed?

Good point, I should have elaborated on this. I'm suggesting the removal of many docstrings from methods from the PrinterEventHandler class because methods like on_send will be called at the same time and with the same arguments as the printcore send callback function. So the functionality is explained in more detail within printrun.printcore.Callback class. Same logic applies to all other methods but for on_init, on_connect and on_disconnect for which the docstring remains. I tried to explain this at the class docstring.

[DivingDuck]

Hi,
Looks like I'm always too late.

I agree to what @neofelis2X wrote. I too think this is an improvement for the code. 👍

I took a look on the refactoring changes and did some tests for the windows versions. Everything runs as expected. The only thing I saw was, that we do not have all logging warnings as translatable strings, but this is something we can do later. (I hadn't translate all strings in the past for this part of the code)

I haven't any real test scenarios for using Printcore directly, so I can't test this. Please let me know if there is something specific I can or should test in this regard.

[rockstorm101]

I took a look on the refactoring changes and did some tests for the windows versions. Everything runs as expected.

May thanks for reviewing.

The only thing I saw was, that we do not have all logging warnings as translatable strings.

True, that might be an easy fix though. I'll have a look. However, I'm unsure whether we should translate warnings and error messages from a library. I would've thought only the user apps (i.e. pronsole and pronterface) require translations. I'm unsure whether other projects actually translate their back-end libraries.

Please let me know if there is something specific I can or should test in this regard.

Unless you run pronterface/pronsole with addons, I don't think there's much more to test. Thanks again :)

[DivingDuck]

I'm fine with the refactoring. Well done.

[neofelis2X]

Approval also from me!