Central logging

[janiversen]

Simplify logging by providing global log function, that does all the handling (including only creating the f string, when we really want to log something).

[knight-of-ni]

@janiversen I just started working with pymodbus when I came across an issue that seems related to #1324.

When I call main.py, to start the http server, python complains with:

  File "/usr/lib/python3.11/site-packages/pymodbus/server/simulator/./main.py", line 103, in get_commandline
    Log.info("Start simulator")
  File "/usr/lib/python3.11/site-packages/pymodbus/logging.py", line 73, in info
    if logging.INFO >= cls.LOG_LEVEL:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '>=' not supported between instances of 'int' and 'str'

In my case the contents of cls.LOG_LEVEL turned out to be the string "INFO", rather than an integer representing a log level of info.

[janiversen]

Seems you called the log start function with a string, and not the integer. Might be worth while to make a check and convert if needed.

[janiversen]

hmmm there must be some confusion. logging.INFO (the default value for —log) is an integer, when I check it,

How did you call main ?

[janiversen]

Never mind #1343 solves your issue, it will soon be merged to dev.

[knight-of-ni]

Thank you for the fast response

[wlcrs]

I'm surprised by the changes in this PR. It breaks the assumption that when you properly configure Pythons logging framework to receive a certain level of logs of 'pymodbus' that they will start appearing in your logs.

Can you switch back to using logger.isEnabledFor instead of keeping a custom variabele in logging.py which you check against? I don't think it's a good idea to have to manually configure the logging for every library that you use.

[janiversen]

we have never used logging.isenabledfor, so I do not know what you want to switch back to.

The local variable we keep in the class is to avoid overhead of calling .getEffectiveLevel() with every log call.

We centralized the log calls in a class a while ago, to make logging more consistent and the calls a lot simpler (log do not like f strings), so you might actually have seen it before.

Anyhow, pull requests are welcome.

[janiversen]

If you talk about the function to activate the logs, it has been there for quite a while, and you actually do not need to call it, to activate logging. You need to call it if you want to submit an issue, because it formats the pymodbus log in a way that enables enables the maintainer to read it automatically.

[wlcrs]

I'm talking about the LOG_LEVEL class variable

I don't understand why it is introduced, as the logging framework is already set up in such a way that it will not process logging calls if there is no handler attached to it (cfr. https://docs.python.org/3/howto/logging.html#logging-flow).

As stated on https://docs.python.org/3/howto/logging.html#optimization logger.isEnabledFor is useful when computing the arguments to be passed in the log message is expensive. So it is the right tool for to prevent unneccessary calls to Log.build_msg.

You already set logger.setLevel in the line above, so the LOG_LEVEL constant is unnecessary and creates unexpected behavior for a Python library.

[janiversen]

let's just say we think that the variable is useful, but looking forward to see a pull request which we can review,