Replace print statements with logging statements#370
Replace print statements with logging statements#370JonasVautherin merged 4 commits intomavlink:mainfrom
Conversation
julianoes
left a comment
There was a problem hiding this comment.
Nice, yes that would be a great change. Do you think the default should be not to log these?
|
I think that the default should be to log the messages, since it is easier for users to discover and silence unwanted log messages rather than discover loggers to enable. After looking at some other libraries like urllib3 it looks like the convention is to not set a log level in the library modules and instead let users select the level when configuring logging for their application. logging.basicConfig(stream=stdout, level=logging.DEBUG) |
|
@qthibeault ok so could we do it similar to urllib3 then? |
|
@julianoes I think that would be the best solution. I will remove the statement to set the level. |
Using urllib3 as an example, users should be able to set the default level of all loggers when configuring logging instead of relying on each library to set the appropriate level.
JonasVautherin
left a comment
There was a problem hiding this comment.
Sounds great! I just need to test it 😊.
Out of curiosity, would you see an easy way to get the mavsdk_server output into that logger? Right now we just ignore it (see here, but it would be great to have the choice.
Create a thread which iterates over the lines in a given pipe and applies logging function to them. Threads are particularly suited to this use case because this is an IO bound operation. When the process is closed, the pipe will run out of lines and the thread will terminate.
|
I added in functionality to log the output of the Initially I tried to create a file-like object to use as |
There was a problem hiding this comment.
Working great for me, e.g. using like this:
diff --git a/examples/takeoff_and_land.py b/examples/takeoff_and_land.py
index 4eda81d..8ec98ce 100755
--- a/examples/takeoff_and_land.py
+++ b/examples/takeoff_and_land.py
@@ -1,6 +1,8 @@
#!/usr/bin/env python3
import asyncio
+import logging
+import sys
from mavsdk import System
@@ -33,5 +35,7 @@ async def run():
await drone.action.land()
if __name__ == "__main__":
+ logging.basicConfig(stream=sys.stdout, level=logging.DEBUG)
+
loop = asyncio.get_event_loop()
loop.run_until_complete(run())
The print statements in the
async_plugin_managermodule can create visual clutter when connecting to multiple systems. Users should be able to decide if those statements are printed by configuring logging on an application level.By default, if logging is configured the mavsdk connection messages should be printed. Users can easily silence them by selecting the appropriate logger and setting the level to anything above DEBUG.
Example: