gmoccapy -add log levels to gmoccapy and getiniinfo #2323

c-morley · 2023-02-04T05:39:38Z

This will allow the user to control the many debug messages sent to the terminal.
It uses qtvcp's logging library so no reinventing the wheel.
Other files should probably be converted to use logging too.

Resolves #2273

hansu · 2023-02-04T12:00:42Z

Really great Chris, thank you! 👍 👍 👍
I also had in mind to use qtvcp's logger for gmoccapy, but I just didn't find the time to implement it.
Can you target the PR to this branch: https://github.com/LinuxCNC/linuxcnc/tree/gmoccapy-3-4-2 ?
Then I can add info to the change log and docs and merge to 2.9.

hansu

Great job in assigning the log levels! I agree in almost most of the cases, however, I have still some suggestions.

And the correct order of log levels is DEBUG, INFO, VERBOSE, WARNING, ERROR, CRITICAL, is that right?

And they can be only set like this, right?

[DISPLAY]
DISPLAY = gmoccapy -q

Maybe it would be good to have a LOG_LEVEL in the DISPLAY section as well.

src/emc/usr_intf/gmoccapy/gmoccapy.py

src/emc/usr_intf/gmoccapy/getiniinfo.py

src/emc/usr_intf/gmoccapy/gmoccapy.py

hansu · 2023-02-04T16:11:28Z

src/emc/usr_intf/gmoccapy/gmoccapy.py

+        logger.setGlobalLevel(logger.DEBUG)
+        LOG.debug('DEBUGGING logging on')
+    elif '-i' in sys.argv:
+        # Log level defaults to INFO, so set lower if in debug mode


Suggested change

# Log level defaults to INFO, so set lower if in debug mode

# Log level defaults to WARNING, so set lower if in debug mode

defaults to WARNING if I get it right?

What about a flag for CRITICAL?

yes i se the comments are wrong - copied from qtvcp and i changed the default level recently.
I'll need to fix them on qtvcp too. thanks

my opinion:
critical - in my mind if you have critical errors you should pretty much be logging then shutting down.
Having a critical log setting would block errors from showing. can't think of any reason you would want to hide errors.

c-morley · 2023-02-04T17:34:41Z

Great job in assigning the log levels! I agree in almost most of the cases, however, I have still some suggestions.

And the correct order of log levels is DEBUG, INFO, VERBOSE, WARNING, ERROR, CRITICAL, is that right?

And they can be only set like this, right?
[DISPLAY]
DISPLAY = gmoccapy -q
Maybe it would be good to have a LOG_LEVEL in the DISPLAY section as well.

verbose, debug, info, warning, error, critical
unfortunately at the moment verbose works but does not say 'verbose' it says 'debug' and is the same color as debug.

LOG_LEVEL is more difficult because the log levels must be set before loading some libraries and before instantiating gmoccapy . Not impossible -just have to add code to parse the INI.
In the current suggestion it is consistent with qtvcp which is nice too.

i don;t know how to change the base branch - could you instruct me?

src/emc/usr_intf/gmoccapy/gmoccapy.py

hansu · 2023-02-04T18:46:51Z

LOG_LEVEL is more difficult because the log levels must be set before loading some libraries and before instantiating gmoccapy . Not impossible -just have to add code to parse the INI. In the current suggestion it is consistent with qtvcp which is nice too.

Yeah but it's fine so. No problem if documented properly.

i don;t know how to change the base branch - could you instruct me?

You'll find the edit button for that at the top of this page:

c-morley · 2023-02-04T18:55:56Z

Base has been changed, thanks !

c-morley · 2023-02-04T19:01:42Z

I could squash together if you would prefer.

hansu · 2023-02-04T19:43:15Z

I could squash together if you would prefer.

Yeah would be nice. Have you any thoughts on my other comments?

c-morley · 2023-02-04T19:52:22Z

I thin I have answered all comments - if I have missed one please ask me again or point them out.
If you are referring to some code changes, say changing from error to warning or such - I have no strong opinion - I just set them to what seemed reasonable in a 10 second look.
Feel free to change them to what ever you feel is best.
I'll squish the branch in a couple minutes.

This will allow the user to control the many debug messages sent to the terminal. It uses qtvcp's logging library so no reinventing the wheel. Other files should probably be converted to use logging too. Ultimately it would be nice if gmoccapy and qtvcp used the same INI parsing code too.

hansu · 2023-02-06T20:59:33Z

@c-morley What does this mean?
[Gmoccapy][INFO] Base log level set to: 20 (logger.py:68)

c-morley · 2023-02-07T01:50:02Z

That is the integer representation of the global log level.
It is in qtvcp/logger.py
I think we should remove it. Opinion?

hansu · 2023-02-07T08:14:13Z

Then it feels a bit redundant, because it says first "INFO logging on" and then this. (Didn't remember if what it prints for default - WARNING).
And using "Base" confused me a bit as well as switching to an integer representation.
So if really redundant, it can be removed. Otherwise use the log level name.

Another thing came into my mind while I had a look at the multi-line log messages. It would be nice if the output can be look like this:

[Gmoccapy][INFO] A long long long long long long long
                 long long message that goes over several lines.

Currently, it's only possible either so

[Gmoccapy][INFO] A long long long long long long long
long long message that goes over several lines.

or so

[Gmoccapy][INFO] A long long long long long long long
[Gmoccapy][INFO] long long message that goes over several lines.

First would be no problem if there were no other messages. For the second it's hard so see how many errors/warning there are actually.
(I mean, yes you can add the correct number of spaces - the logging domain would probably not change, so that would work, but inconvenient and a bit ugly 🙃)
I thought about something like this, so a function "newline" that adds as many spaces as the last prefix needed:

LOG.info("A long long long long long long long")
LOG.newline("long long message that goes over several lines.")

Dou you think that is a reasonable feature?

c-morley · 2023-02-08T01:37:55Z

for LOG.newline()to be useful one would need LOG.info_newline() plus all the other log levels.
I am still having trouble with getting extra log levels to work properly.
you could use:
LOG.info("A long long long long long long long line\n and the rest of the long line line.")

c-morley · 2023-02-08T01:38:27Z

should be 8 spaces between \n and the 'and'

hansu · 2023-02-08T08:16:42Z

for LOG.newline()to be useful one would need LOG.info_newline() plus all the other log levels.

I thought about a static class variable that "remembers" the last used log level.

you could use:
LOG.info("A long long long long long long long line\n and the rest of the long line line.")
should be 8 spaces between \n and the 'and'

Yeah that's what I was meaning in the sentence inside the parantheses.

hansu · 2023-02-09T19:28:37Z

Let's skip this. This seems to be too much work for a bit improvement in readability. Because in the log file there has to be added number of spaces...

c-morley requested a review from hansu February 4, 2023 05:39

hansu added the affects gmoccapy label Feb 4, 2023

hansu reviewed Feb 4, 2023

View reviewed changes

src/emc/usr_intf/gmoccapy/gmoccapy.py Outdated Show resolved Hide resolved

c-morley changed the base branch from 2.9 to gmoccapy-3-4-2 February 4, 2023 18:55

c-morley force-pushed the gmoccapy_logging branch from ff9fa22 to 2e8e43e Compare February 4, 2023 19:57

hansu force-pushed the gmoccapy-3-4-2 branch from 452e032 to d5abb73 Compare February 4, 2023 21:33

hansu merged commit e8a2e2d into gmoccapy-3-4-2 Feb 4, 2023

c-morley deleted the gmoccapy_logging branch February 4, 2023 22:09

hansu mentioned this pull request Feb 9, 2023

GMOCCAPY 3.4.2 #2330

Merged

hansu mentioned this pull request Jun 30, 2025

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

Draft

	# Log level defaults to INFO, so set lower if in debug mode
	# Log level defaults to WARNING, so set lower if in debug mode

gmoccapy -add log levels to gmoccapy and getiniinfo #2323

gmoccapy -add log levels to gmoccapy and getiniinfo #2323

Uh oh!

Conversation

c-morley commented Feb 4, 2023 • edited by hansu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hansu commented Feb 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hansu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hansu Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

c-morley Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

c-morley commented Feb 4, 2023

Uh oh!

Uh oh!

hansu commented Feb 4, 2023

Uh oh!

c-morley commented Feb 4, 2023

Uh oh!

c-morley commented Feb 4, 2023

Uh oh!

hansu commented Feb 4, 2023

Uh oh!

c-morley commented Feb 4, 2023

Uh oh!

hansu commented Feb 6, 2023

Uh oh!

c-morley commented Feb 7, 2023

Uh oh!

hansu commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

c-morley commented Feb 8, 2023

Uh oh!

c-morley commented Feb 8, 2023

Uh oh!

hansu commented Feb 8, 2023

Uh oh!

hansu commented Feb 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

c-morley commented Feb 4, 2023 •

edited by hansu

Loading

hansu commented Feb 4, 2023 •

edited

Loading

hansu commented Feb 7, 2023 •

edited

Loading