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

Feature Request: Management of configuration that doesn't belong to the Machine #1482

Open
alexanderwiller opened this issue Sep 5, 2022 · 3 comments

Comments

@alexanderwiller
Copy link
Contributor

Feature Request

Describe the Feature

The feature should provide a place for storing configuration that is not machine configuration.

Machine configuration should be defined as anything that relates to the physical capabilities of the machine, directly or indirectly. I think this is best illustrated by all items that are now to be found under ReferenceMachine in the setup tree.

Not machine configuration is everything else, some examples include:

  • Application language and units
  • Application behavior (Script engine pooling, ...)
  • Explicit UI configuration (e.g. not window positions, but speed slider limit, table selection behavior, camera zoom increments, ...)
  • Macros (don't exist yet, see Additional Pick and Place buttons. #432 (comment))

I would coin the term Application configuration for this.

How Will It Look?

grafik

The feature will use the same AbstractModelObject base class as all machine configuration objects and will be configured the same way (PropertySheetHolder etc.).

A separate configuration file, application.xml, will be used for storing the application settings. This is due to the structure of the machine.xml not really allowing any additional config next to, not subordinate to, the machine element. Also, it seems to be cleaner architecturally.

For the config tree, an invisible root node is added that contains both the machine and the application configuration.

Code-wise, it could be structured like this:
grafik

Im not too sure with this point, its just a first draft to make it work and produce the mock-up (that isn't really a mock-up anymore). Abstract classes may or may not be used, depending on if different implementations can be expected (e.g. not for Application, but maybe for ScriptMacro, PredefinedActionMacro, ActuatorMacro, ...). Here I would need some advice, maybe after making available a first draft in my branch, to ensure that clean separation between model and whatever else (I'm not an expert at these things but just hacking away) isn't watered down.

Why Do We Need It?

1. What problem does it solve?

It feels not right structure-wise to add a configuration like script engine caching, or even worse, speed slider low limit, to the ReferenceMachine class and Machine interface.

The class/interface gets bloated by this and those configuration items are not related to the physical capabilities of the machine, but to the way of interacting with it or how the application behaves internally. They are even useful to retain if one does create a new machine.xml.

Additionally, system units, language preferences and UI behavior currently are stored in the system registry (or some equivalent mechanism), making these settings not portable between systems by just copying the config folder.

2. Is it useful for everyone, or does it only solve a problem for your machine?

For myself, I wouldn't go to the efforts doing this, this proposal should improve structural representation of the various classes of settings that already exist and provide clean infrastructure for future additions to OpenPnP.

@markmaker
Copy link
Collaborator

Hi @alexanderwiller,

It feels not right structure-wise to add a configuration like script engine caching...

I can follow you that most things should be stored in one of the usual .xml files, instead of (as it is today) in java.util.prefs.Preferences through the org.openpnp.model.Configuration.prefs. I always hate it that restoring a machine.xml does not restore these properties, and that they are not visible/editable when users send their config.

Likewise, I agree, the Machine should not become the cesspool for "lost+found properties".

But having two roots Application and Machine does not feel right either. If at all, we should probably expose the already existing Configuration object at the top and rename the GUI tab to "Configuration". The Machine would become a child of Configuration (could be multiple Machines one day). As a new sibling of Machine, you could also expose the already existing Scripting object, and add the pooling switch there.

But this would require huge code changes! You would probably need to add a new configuration.xml file, but with just the new "overarching" settings, because it would be very disruptive to discontinue the machine.xml, and handle migration.

For a less intrusive change the Machine can remain the root of the "Machine Setup" (tab title), and the Scripting object could be added as a new child of Machine. Likewise, other (future) settings objects can be added like this.

In a way this is already the case today: the Feeders node is actually not a part of the machine.xml but separate, the Job Processor and Vision nodes could arguably also be siblings of the Machine, rather than children. Obviously, this could also be taken as an argument for the full remodeling.

... or even worse, speed slider low limit, to the ReferenceMachine class and Machine interface.

Why? IMHO, the machine is the perfect place for the speed slider low limit. It is the overall machine speed that is governed, i.e. across all drivers and axes.

_Mark

@alexanderwiller
Copy link
Contributor Author

Hey @markmaker

I want to first try to establish the points we agree on:

  • It would be desirable to abandon any configuration store that is not in the .openpnp2 directory in the form of XML files, except maybe for window positions and similar things (future custom toolbars would still go into XML).
  • There is a need for some kind of container structure for those configuration items that don't fit into the machine or in any other existing class, like localization settings, macros and custom toolbars.
  • The speed slider low limit can be a property of the machine, but then I will rename it to speedLimitLowPercent and speedLimitHighPercent. The high limit would be helpful for the commissioning of the machine, you can first set it low, then increase it when everything is running properly to protect yourself and the machine from accidental high-speed operation.
  • The machine.xml should be kept as-is, because both as a file representing something very specific and as a configuration root node its very fitting, also any migration would be disruptive.

Please correct me if I got anything wrong here.


I don't get this part:

In a way this is already the case today: the Feeders node is actually not a part of the machine.xml but separate, the Job Processor and Vision nodes could arguably also be siblings of the Machine, rather than children. Obviously, this could also be taken as an argument for the full remodeling.

The feeders can be found in the machine.xml. Do you mean parts.xml?

For the job processor, I think it belongs into the machine, as a change in job processing can lead to a major machine behavior change. Consider having a machine that has a tight max clearance between the nozzle and the PCB, it could depend on the job order to not crash into already placed parts. When you load a different machine.xml because you are using another machine, you would except that the job executes flawlessly, not that some application setting causes it to fail.

I also already thought about vision and think the settings are reasonably portable as long as the camera is somewhat compatible. I don't know how this would interact with gang vision as suggested in another issue, but think it wouldn't be a problem. The machine knows nozzle spacing and camera positions, the vision settings are just used per (part, camera) combination. So Vision probably could be a sibling, instead of a child of Machine.

But I digress, these are some very specific questions already, just wanted to address them.


Regarding the probably biggest topic of discussion, how any new settings items that are not directly related to the machine (Scripting, Macros, ...) can be represented, I don't fully understand in which way having two or more roots doesn't feel right.

You already outlined the two options that I would see as well:

  • Put them under the Machine as children, because thats the only thing the current structure allows
  • Put them next to the Machine as siblings, where they structurally belong

I woudl argue that only the second option represents an actual improvement, which is the point of this issue.

I could see it from the GUI tree perspective, where code-wise, its just a root tree node class implementing PropertySheetHolder and having Machine and Application (and maybe Vision and Macros if you don't want to nest them) as children. UI-wise, its two (or more) top-level elements instead of one. I don't really see any issue with these two aspects, do you?

Considering the Configuration class, I just copy-pasted what was already there, like the loading and saving of the other configuration files. The class now has an extra property, Application, right next to Machine and the other properties (Vision etc.).

Data model-wise, I'd say that having an Application class would be a sufficiently generic roof under which things like scripting, macros, toolbars fit. It's not well-represented in the Machine and there isn't any other existing container that can be used.


I like the idea of having a Configuration class extending AbstractModelObject and being an XML file just containing paths to other config files, because of these points:

  • The tree would not need a synthetic root node anymore
  • You can easily specify a different configuration file for any "component" without command line flags (but can still offer flags for every component)
  • Configuration file paths aren't hard-coded in the Configuration class anymore, except for configuration.xml (still overrideable)
  • Migration is more or less trivial, just create the configuration.xml with all the paths that OpenPnP would load its configs from if that file doesn't exist yet

I agree that this overall endeavour would require a lot of code changes, but of a not too difficult type, its basically restructuring legwork, not something hugely complex on its own. As I've invested quite a bit of effort into this topic already, I'm open to trying to implement any changes that we agree upon.

Alex

@markmaker
Copy link
Collaborator

establish the points we agree on

Yes, we do agree on those.

Feeders node

Haha, I was mistaken. The fact that the feeders are also available separately on the Feeders tab threw me. Just momentarily took them the same way as Parts, Packages and Vision which have their own .xml. Funny how the brain associates sometimes.

Put them next to the Machine as siblings, where they structurally belong

I disagree that this is so clear-cut. It could be well argued that the Machine is the defining root object for one instance of OpenPnP, and everything else is owned by it, including the application being attached to control it. This is just a more or less arbitrary design decision.

I don't fully understand in which way having two or more roots doesn't feel right.

Well, that's more of a stomach feeling, professional experience after 35 years of developing software. I always liked the UNIX idea that everything is a file and that there is a single file hierarchy. In comparison, the clunky drive letters on WinDOS are an abomination. Same thing with Java's Object OOP type system root as opposed to C++'s mess. Or take a DOM. You get a clear and complete ownership hierarchy, and some very elegant functionality can be achieved using bubbling and/or recursion, taking topological order into account.

But frankly, this discussion is a bit beyond the scope of what I personally think is very useful for OpenPnP, and worthy of my spare time (and yours). There are so many "down to earth" things that you could help improve in OpenPnP (JobProcessor etc.) that would help users day after day, big time. The sort of architectural perfectionism we are talking here, comes way down the list for me, given this is well established and proven code that nobody really complained about. No offense! 😇

Maybe Jason, being the architect of it all, has a different take on your proposals. I certainly won't stand in the way 😆

_Mark

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

No branches or pull requests

2 participants