Gmoccapy - change spindle => spindle_0 in *.ini files

[zz912]

RIP installation
LCNC 2.10
GUI Gmoccapy

How did it start?
I had a clean configuration from PncConf and after starting LCNC it gave me these error messages:

[Gmoccapy.QTVCP.QT_ISTAT][WARNING]  INI Parsing Error, No MIN_SPINDLE_0_SPEED Entry in DISPLAY, Using: 100 (qt_istat.py:676)
[Gmoccapy.QTVCP.QT_ISTAT][WARNING]  INI Parsing Error, No MAX_SPINDLE_0_SPEED Entry in DISPLAY, Using: 2500 (qt_istat.py:676)
[Gmoccapy.QTVCP.QT_ISTAT][WARNING]  INI Parsing Error, No MAX_SPINDLE_0_OVERRIDE Entry in DISPLAY, Using: 1 (qt_istat.py:676)
[Gmoccapy.QTVCP.QT_ISTAT][WARNING]  INI Parsing Error, No MIN_SPINDLE_0_OVERRIDE Entry in DISPLAY, Using: 0.5 (qt_istat.py:676)

So I edited spindle => spindle_0 in the INI files.
The problem was that the QT_ISTAT error messages disappeared, but the Gmoccapy error messages appeared.
So I had to edit Gmoccapy to require spindle_0.
Finally, I edited PncConf to generate spindle_0 for Gmoccapy.
While I was playing with PncConf, I added more comments to the generated INI files.

[gmoccapy]

Why did you need to change all the Min and Max Spindle stuff?
Should have worked bevore without errors imho

In gmoccapy.Py you introduced a lot of print statements, why?
Are these ones debug messages during yoor tests?

Norbert

[hansu]

[Zdeněk]

So I edited spindle => spindle_0 in the INI files.

[Norbert]

Why did you need to change all the Min and Max Spindle stuff?

For that see #3238, also my working branch https://github.com/hansu/linuxcnc/commits/gmoccapy-3-5-1-ini-values especially hansu@e0359dc

Sorry this got stuck for a while in my pipeline.

[zz912]

To Hans:

Can you please explain to everyone what QTVCP.QT_ISTAT is and what it is for?

It will help me explain what, why and how I did it, or edit my PR.

I thought QTVCP.QT_ISTAT was something new, so I adapted everything to it so that it doesn't throw error messages.

When do you plan to merge your gmoccapy-3-5-1-ini-values ​​branch?
It would probably be better to edit this PR to your branch?

Or should I close this PR and you edit your branch inspired by this PR?

[Norbert]

In gmoccapy.Py you introduced a lot of print statements, why?

I introduced a lot of print statements in src/emc/usr_intf/pncconf/build_INI.py not in gmoccapy.py

Generated ini file before:

Generated ini file after:

It is comments from your file https://github.com/LinuxCNC/linuxcnc/blob/master/configs/sim/gmoccapy/gmoccapy_4_axis.ini

Zdeněk

[hansu]

Can you please explain to everyone what QTVCP.QT_ISTAT is and what it is for?

Because the logger from QTVCP is used here and it uses this module, here is a bit about that:
#2323
#3126
It also reads the Ini values, but expect different ones, so therefore the Errors messages (which are only warnings).

You can add additional values with "MIN_SPINDLE_0_SPEED" and so on to suppress these warnings.

When do you plan to merge your gmoccapy-3-5-1-ini-values ​​branch?

I think at some time in the autumn this year. It is a bit work because I have to edit a lot of INI files. And it also touches some other GUIs.

It would probably be better to edit this PR to your branch?

I think it would be the best if you wait until my branch is merged and then rebase your branch.

[Norbert]

Should have worked bevore without errors imho

It still works, but only throws warning since gmoccapy uses the qtvcp logger (#2323)

[zz912]

I will wait for the release of 3.5.1

[hansu]

I will wait for the release of 3.5.1

3.5.1 is ready, but the INI value stuff will not make it into 3.5.1 unfortunately even though the branch name suggests that

[Sigma1912]

It also reads the Ini values, but expect different ones, so therefore the Errors messages (which are only warnings).

I don't understand why a logger module autonomously throws errors about ini file entries. Wouldn't we use the logger module to log errors about entries missing for gmoccapy?

[Sigma1912]

Not sure if it is such a great idea to mix qtvcp into gladevcp:

https://github.com/hansu/linuxcnc/blob/master/lib/python/gladevcp/core.py#L13
https://github.com/hansu/linuxcnc/blob/master/lib/python/gladevcp/core.py#L16

[zz912]

[Sigma]

I don't understand why a logger module autonomously throws errors about ini file entries. Wouldn't we use the logger module to log errors about entries missing for gmoccapy?

The situation is not as simple as it seems.

I think Hans is doing a good job trying to unify the INI parameters of all GUIs. Unfortunately, this creates problems that need to be solved.

Now, we have 5 problematic INI parameters:
MAX_SPINDLE_OVERRIDE
MIN_SPINDLE_OVERRIDE
DEFAULT_SPINDLE_SPEED
MIN_SPINDLE_SPEED
MAX_SPINDLE_SPEED

In version 2.7 and older LCNC could only handle one spindle.
Stuff about spindles was called spindle.

In version 2.8 and newer LCNC could handle N spindles.
Stuff about spindles was called spindle_N.

In Gmoccapy (and also in other GUIs) only the most necessary things were rewritten to make it work. "spindle" was rewritten to the first spindle "spindle_0".

Unfortunately, the names of the parameters in the INI files related to spindles were evaluated as not necessary to change.

Then when Hans did an ISTAT check, the check wanted parameter names with "spindle_0". With this Pull Request I wanted to solve only this unification.

When I was dealing with this issue I found out:

1. Since we already have multispindle LCNC, GUI should also be multispindle and spindle parameters should be in array spindle[N]
2. ISTAT requires 5 parameters related to spindle
   Gmoccapy requires 3 parameters related to spindle
3. DEFAULT_SPINDLE_SPEED parameter sets default speed only when Gmoccapy is first started, then default speed is set from *.pref file. For a person who doesn't know, it can be confusing, so I at least pointed it out with a comment.
   I don't want to solve these problems in this PR. I want make small steps.

So we use the logger module to log errors about 3 entries missing for gmoccapy.

[zz912]

Not sure if it is such a great idea to mix qtvcp into gladevcp:

https://github.com/hansu/linuxcnc/blob/master/lib/python/gladevcp/core.py#L13 https://github.com/hansu/linuxcnc/blob/master/lib/python/gladevcp/core.py#L16

LinuxCNC is already the Tower of Babel.

[c-morley]

Not sure if it is such a great idea to mix qtvcp into gladevcp:

https://github.com/hansu/linuxcnc/blob/master/lib/python/gladevcp/core.py#L13 https://github.com/hansu/linuxcnc/blob/master/lib/python/gladevcp/core.py#L16

Can you explain why? Qtvcp and Gladevcp also both use hal_glib, should we recreate a qthal_ glib too? I think people are getting upset about a name. The idea is to get less duplication of code. Hans and I have discussed bit about this. Thete are plenty of ways to improve the ini error reporting. I beleive the first thing tp go is to get qtvcp and gladevcp/gmoccapy to use the same core ini reader. But the code i added that pulled Istat in was for gladevcp panels not Gmoccapy.

[Sigma1912]

Thanks for your comments.
Avoiding code duplication is not wrong but trying to make everything reusable comes at a cost of ever increasing complexity.
In my opinion that is just terrible for future maintainability. I wish we would start to compartmentalize these different GUIs instead of trying to bake everything into an ever increasing stew.
Gmoccapy, which is, or used to be, a GTK gui, now throws error messages from a QT module about missing ini entries that Gmoccapy does not even use. The solution here apparently is to add these unused entries into the ini file. On top of that maintaining Gmoccapy now also requires maintaining pyqt as well as gtk.

[c-morley]

As I said there is work we can do about error/warning messages.
This was part of a push to get there:
https://linuxcnc.org/docs/devel/html/gui/gui-dev-reference.html

There is little sense in creating two modules that do approximately exactly the same thing.
The other nice thing about one module that parses/qualifies INI files, that all the GUis that use it are more easily switches between as they don't use slightly different text for the same thing (exactly what we are talk about) - easier for the users. I think this is even a better reason then just not having duplicate modules.

Have you looked at qt_istat.py? There are no PyQt libraries used in it - don''t get hung up on a name.

To sumarize:
-It would be better to parse/qualify common INI settings in a common module.
better: less code, maintainability and easier for user to screen switch.

-Don't get too hung up in a name (and we could surely change that name anyways)

-The ini/action changes were made for Gladevcp not Gmoccapy, but since Gmoccapy uses Gladevcp, we must work around them - probably better to work with them.

[zz912]

I studied:
http://linuxcnc.org/docs/devel/html/gui/gui-dev-reference.html

• there is no note about ISTAT.
• I found new parameter SPINDLE_INCREMENT - what is it? Why is not SPINDLE_0? Why is not checked by ISTAT?
  I prefer remove it from manual page.

I agree with parse/qualify common INI settings in a common module. BUT Gmoccapy uses the same parameters from the INI file and the same parameters can be set in the setup page. These are then saved in the *.pref file. So we have a duplicate source of some parameters.

What should the future of Gmoccapy be?

• Use duplicate INI parameters only for the first Gmoccapy load and then read them from *.pref? This is the current state. But I think it needs to be pointed out.
• Remove duplicate parameters from setup page and use the parameters from INI file only?
• Save parameter changes from the setup page directly to the INI file? (I don't know if this is technically possible)
• Remove duplicate parameters from INI file and make an exception for Gmoccapy in ISTAT
• other

I think qt_istat.py could be renamed to istat.py and moved here:
linuxcnc/lib/python/

qt in name make me unhappy too.

[hansu]

I found new parameter SPINDLE_INCREMENT - what is it? Why is not SPINDLE_0?

Because it's a step size for all spindles like the [DISPLAY] INCREMENTS is for all linear axes.

BUT Gmoccapy uses the same parameters from the INI file and the same parameters can be set in the setup page. These are then saved in the *.pref file. So we have a duplicate source of some parameters.

Yeah some. But that's a Gmoccapy-only thing

Save parameter changes from the setup page directly to the INI file? (I don't know if this is technically possible)

No, we don't want this

[zz912]

I found new parameter SPINDLE_INCREMENT - what is it? Why is not SPINDLE_0?

Because it's a step size for all spindles like the [DISPLAY] INCREMENTS is for all linear axes.

I don't think it's the same. Many machines have a main spindle with a range of 50 - 1500 rpm and an auxiliary spindle with a range of 1000 - 100,000 rpm. What increment would you choose here? To make sense, the increment would have to be in percent. However, I still can't imagine using this parameter. (This is because I don't know the other GUIs)

BUT Gmoccapy uses the same parameters from the INI file and the same parameters can be set in the setup page. These are then saved in the *.pref file. So we have a duplicate source of some parameters.

Yeah some. But that's a Gmoccapy-only thing

I am interested ONLY about Gmoccapy.

Save parameter changes from the setup page directly to the INI file? (I don't know if this is technically possible)

No, we don't want this

For the standardization of LCNC it would be ideal to remove duplicate parameters from the setup page. Then it would be possible to debug LinuxCNC with all GUIs the same. But I think Norbert will not like it.

[gmoccapy]

I am a friend of unifying code stuff.
If we can get rid of Settings and unify them in INI Parameters, that would be fine, as long as we stay compatible to previous versions of have an update script.

IMHO mixing QT and GTK Stuff is not the way to go.
I agree with Sigma1912, that related to maintanance it is better to have the same code in two files to keep stuff separated, like in this case a GTK based and a QT based GUI.

[c-morley]

So the problem is the name then? qt_istat.py has no PyQt libraries in it.
The name can be changed, the file moved, if that is important to all of you.
At that point the warning messages will still show, so would still need to discuss that.

[Sigma1912]

It's not just the name.

It's also location:

from hal_glib import GStat
from qtvcp.qt_istat import _IStat as IStatParent

# Set up logging
from qtvcp import logger

It's also about paths used in the code (eg 'qt_istat.py'):

43:    IMAGEDIR = os.path.join(BASE, "share", "qtvcp", "images")
73:        self.LIB_PATH = os.path.join(BASE, "share", "qtvcp")
107:        self.QTVCP_LOG_HISTORY_PATH = self.INI.find('DISPLAY', 'LOG_FILE') or '~/qtvcp.log'

and function names:

783:    def get_qt_filter_extensions(self):

comments:

76:        # this is updated in qtvcp.py on startup
542:        # tab style for qtvcp tab. style is used everywhere
565:            # check for duplicate names if qtvcp panels
581:            # find qtvcp embedded - because they are added directly rather then x11 embedding

Code that is to be shared by Qt AND non-QT based GUIs should not

• use the Qt library
• have 'qt' in the file-, class-, function-, variablenames, comments, etc.
• be in the folder with the stuff that is only meant to be used by QT based GUIs .

Don't get me wrong, I realize that you have put tremendous amounts of time and effort into the Gui section of LinuxCNC. It has CMorley written all over but with this being in the source code we need to pay extra attention to maintainability for future developers. Most of the time we like to just make things work and not so much cleaning things up.

Thank you.

[zz912]

I would like to propose that the debate regarding ISTAT be moved somewhere else.
This Pull Request is about changing the name of the parameters "spindle" => "spindle_0" in *.ini files. This Pull Request would like to merge it, whether ISTAT will be any, or not in Gmoccapy at all.

I have also created an Issue regarding duplicate parameters in Gmoccapy.
#3497