intelligent-agent / redeem

Firmware for Replicape
http://wiki.thing-printer.com/index.php?title=Redeem
GNU General Public License v3.0
36 stars 44 forks source link

Consistency issue: M116 calls GCodeProcessor.resolve() #85

Open yomgui1 opened 6 years ago

yomgui1 commented 6 years ago

description:

M116 executes sub-gcode by calling GCodeProcessor.resolve before GCodeProcessor.execute This is not how all other gcodes proceed. What's the correct way?

branch: 2.2.x (but others can be impacted)

yomgui1 commented 6 years ago

seems that all other commands are incorrect: GCodeProcessor.execute() starts by checking if the command is resolved and call resolve on it it not, but it raises a warning before. So all other commands are raising this warning at each call (TBC).

Wackerbarth commented 6 years ago

We will have to ask @ThatWileyGuy about the intent. I know that M116 is special because of the way it interacts with motion commands. However, when I first saw this, I thought that the intention was to convert string definitions (macros) into a list of "command" objects. And I think that the corresponding conversion was already accomplished for "primitive" commands. The item looked up doesn't need conversion.

But this whole "resolve" doesn't quite make sense to me. It seems that we should know about any required resolution when the command is initially loaded into the system (through configuration, or otherwise) and that we could do the resolution "then and there" and not be required to be testing for every command.

I would also suggest that although we are "paying the price" for using python, we are not taking advantage of its power in terms of objects and classes. Somewhat like using a big lorry to move a large number of small items, but doing so only one item at a time.

ThatWileyGuy commented 6 years ago

I added the concept of "resolving" gcodes when I added the sync events to synchronize the various gcode queues. At the time, the problem I needed to solve is that a sync event effectively executes as multiple different gcodes - for example, a code to make the synchronous queue wait and a code to make the asynchronous queue complete the wait. These codes also need to carry enough context with them that we can link specific pairs together even if there are multiple sync events in the queue.

My solution of "resolving" the codes means that every incoming gcode is "resolved" to its handler and a reference to the handler gets dropped in the gcode. For the synchronization codes, I just create a few custom handlers that carry all the context I need and put those in the same place. This is necessary for the synchronization codes to work, but not really needed for the normal codes. I did it for both at the same time because repeating the same gcode-name-to-handler lookup over and over seemed silly for the normal codes.

yomgui1 commented 6 years ago

@ThatWileyGuy ok, the reason seems legitimate, but I recommend to use a better OOP style (and benefit of Python OO programming) to separate async from sync gcode behaviours than let the programmer guesses how to deal with the API (should a call resolve or note?). If we add more and more special cases/behaviours like that it will be a nightmare to maintain.