From 48851470aec6fe57d332ea1930f172ac63e5bbff Mon Sep 17 00:00:00 2001 From: Banks T Date: Tue, 28 Jul 2020 21:20:54 -0400 Subject: [PATCH] Various logging cleanups (#65) * Various logging cleanups * Move device info to string, apply spotless --- .../src/main/java/org/photonvision/Main.java | 9 +++-- .../common/configuration/ConfigManager.java | 2 +- .../common/dataflow/DataChangeService.java | 2 +- .../photonvision/common/logging/Logger.java | 22 +++++++++- .../common/util/TimedTaskManager.java | 6 +-- .../server/UIOutboundSubscriber.java | 2 - .../vision/pipe/impl/Calibrate3dPipe.java | 3 +- .../vision/processes/PipelineManager.java | 1 - .../vision/processes/VisionModule.java | 10 +---- .../vision/processes/VisionSourceManager.java | 40 ++++++++----------- 10 files changed, 49 insertions(+), 48 deletions(-) diff --git a/photon-server/src/main/java/org/photonvision/Main.java b/photon-server/src/main/java/org/photonvision/Main.java index bbe542d74..bf036fa97 100644 --- a/photon-server/src/main/java/org/photonvision/Main.java +++ b/photon-server/src/main/java/org/photonvision/Main.java @@ -40,14 +40,15 @@ public class Main { public static final int DEFAULT_WEBPORT = 5800; public static void main(String[] args) { - var logLevel = PhotonVersion.isRelease ? LogLevel.ERROR : LogLevel.TRACE; + boolean isRelease = !PhotonVersion.isRelease; // Hack!!!! Until PhotonVersion script fixed + var logLevel = isRelease ? LogLevel.INFO : LogLevel.DEBUG; Logger.setLevel(LogGroup.Camera, logLevel); Logger.setLevel(LogGroup.WebServer, logLevel); Logger.setLevel(LogGroup.VisionModule, logLevel); Logger.setLevel(LogGroup.Data, logLevel); Logger.setLevel(LogGroup.General, logLevel); - logger.info("Logging initialized!"); + logger.info("Logging initialized in " + (isRelease ? "Release" : "Debug") + " mode."); logger.info( "Starting PhotonVision version " @@ -60,7 +61,7 @@ public class Main { } catch (Exception e) { logger.error("Failed to load native libraries!", e); } - logger.info("Native libaries loaded."); + logger.info("Native libraries loaded."); ConfigManager.getInstance(); // init config manager NetworkManager.getInstance().initialize(false); // basically empty. todo: link to ConfigManager? @@ -75,7 +76,7 @@ public class Main { for (var src : sources) { var usbSrc = (USBCameraSource) src; collectedSources.put(usbSrc, usbSrc.configuration.pipelineSettings); - logger.trace( + logger.debug( () -> "Matched config for camera \"" + src.getFrameProvider().getName() diff --git a/photon-server/src/main/java/org/photonvision/common/configuration/ConfigManager.java b/photon-server/src/main/java/org/photonvision/common/configuration/ConfigManager.java index 841ebc2df..6b4db3670 100644 --- a/photon-server/src/main/java/org/photonvision/common/configuration/ConfigManager.java +++ b/photon-server/src/main/java/org/photonvision/common/configuration/ConfigManager.java @@ -224,7 +224,7 @@ public class ConfigManager { driverModeFile.toAbsolutePath(), DriverModePipelineSettings.class); } catch (JsonProcessingException e) { logger.error("Could not deserialize drivermode.json! Loading defaults"); - logger.trace(Arrays.toString(e.getStackTrace())); + logger.debug(Arrays.toString(e.getStackTrace())); driverMode = new DriverModePipelineSettings(); } if (driverMode == null) { diff --git a/photon-server/src/main/java/org/photonvision/common/dataflow/DataChangeService.java b/photon-server/src/main/java/org/photonvision/common/dataflow/DataChangeService.java index e2dedfb3c..65148319e 100644 --- a/photon-server/src/main/java/org/photonvision/common/dataflow/DataChangeService.java +++ b/photon-server/src/main/java/org/photonvision/common/dataflow/DataChangeService.java @@ -76,7 +76,7 @@ public class DataChangeService { if (!subscribers.addIfAbsent(subscriber)) { logger.warn("Attempted to add already added subscriber!"); } else { - logger.trace( + logger.debug( () -> { var sources = subscriber.wantedSources.stream() diff --git a/photon-server/src/main/java/org/photonvision/common/logging/Logger.java b/photon-server/src/main/java/org/photonvision/common/logging/Logger.java index 4a0e3070f..8fbda8ba5 100644 --- a/photon-server/src/main/java/org/photonvision/common/logging/Logger.java +++ b/photon-server/src/main/java/org/photonvision/common/logging/Logger.java @@ -141,12 +141,25 @@ public class Logger { } } + private void log(String message, LogLevel messageLevel, LogLevel conditionalLevel) { + if (shouldLog(conditionalLevel)) { + log(message, messageLevel, group, className); + } + } + private void log(Supplier messageSupplier, LogLevel level) { if (shouldLog(level)) { log(messageSupplier.get(), level, group, className); } } + private void log( + Supplier messageSupplier, LogLevel messageLevel, LogLevel conditionalLevel) { + if (shouldLog(conditionalLevel)) { + log(messageSupplier.get(), messageLevel, group, className); + } + } + public void error(Supplier messageSupplier) { log(messageSupplier, LogLevel.ERROR); } @@ -155,9 +168,16 @@ public class Logger { log(message, LogLevel.ERROR); } + /** + * Logs an error message with the stack trace of a Throwable. The stacktrace will only be printed + * if the current LogLevel is TRACE + * + * @param message + * @param t + */ public void error(String message, Throwable t) { log(message, LogLevel.ERROR); - log(convertStackTraceToString(t), LogLevel.ERROR); + log(convertStackTraceToString(t), LogLevel.ERROR, LogLevel.TRACE); } public void warn(Supplier messageSupplier) { diff --git a/photon-server/src/main/java/org/photonvision/common/util/TimedTaskManager.java b/photon-server/src/main/java/org/photonvision/common/util/TimedTaskManager.java index 775630ae2..4206f1aff 100644 --- a/photon-server/src/main/java/org/photonvision/common/util/TimedTaskManager.java +++ b/photon-server/src/main/java/org/photonvision/common/util/TimedTaskManager.java @@ -42,8 +42,7 @@ public class TimedTaskManager { Thread thread = defaultThreadFactory.newThread(r); thread.setUncaughtExceptionHandler( (t, e) -> { - logger.error("TimedTask threw uncaught exception!"); - e.printStackTrace(); + logger.error("TimedTask threw uncaught exception!", e); }); return thread; } @@ -75,8 +74,7 @@ public class TimedTaskManager { } if (throwable != null) { - logger.error("TimedTask threw uncaught exception!"); - throwable.printStackTrace(); + logger.error("TimedTask threw uncaught exception!", throwable); // Restart the runnable again execute(runnable); } diff --git a/photon-server/src/main/java/org/photonvision/server/UIOutboundSubscriber.java b/photon-server/src/main/java/org/photonvision/server/UIOutboundSubscriber.java index f02dbe0a9..8290153a4 100644 --- a/photon-server/src/main/java/org/photonvision/server/UIOutboundSubscriber.java +++ b/photon-server/src/main/java/org/photonvision/server/UIOutboundSubscriber.java @@ -51,7 +51,6 @@ class UIOutboundSubscriber extends DataChangeSubscriber { switch (thisEvent.updateType) { case BROADCAST: { - // logger.debug("Broadcasting message"); if (event.data instanceof HashMap) { var data = (HashMap) event.data; socketHandler.broadcastMessage(data, null); @@ -62,7 +61,6 @@ class UIOutboundSubscriber extends DataChangeSubscriber { } case SINGLEUSER: { - // logger.debug("Sending single user message"); if (event.data instanceof Pair) { var pair = (SocketHandler.SelectiveBroadcastPair) event.data; socketHandler.broadcastMessage(pair.getLeft(), pair.getRight()); diff --git a/photon-server/src/main/java/org/photonvision/vision/pipe/impl/Calibrate3dPipe.java b/photon-server/src/main/java/org/photonvision/vision/pipe/impl/Calibrate3dPipe.java index ea35290d0..a21e745ab 100644 --- a/photon-server/src/main/java/org/photonvision/vision/pipe/impl/Calibrate3dPipe.java +++ b/photon-server/src/main/java/org/photonvision/vision/pipe/impl/Calibrate3dPipe.java @@ -20,7 +20,6 @@ package org.photonvision.vision.pipe.impl; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import org.opencv.calib3d.Calib3d; import org.opencv.core.*; @@ -97,7 +96,7 @@ public class Calibrate3dPipe + new ObjectMapper().writeValueAsString(distortionCoefficientsMat) + "\n"); } catch (JsonProcessingException e) { - logger.error(Arrays.toString(e.getStackTrace())); + logger.error("Failed to parse calibration data to json!", e); } // Create a new CameraCalibrationCoefficients object to pass onto SolvePnP double[] perViewErrorsArray = diff --git a/photon-server/src/main/java/org/photonvision/vision/processes/PipelineManager.java b/photon-server/src/main/java/org/photonvision/vision/processes/PipelineManager.java index 693c460e1..950db0ac9 100644 --- a/photon-server/src/main/java/org/photonvision/vision/processes/PipelineManager.java +++ b/photon-server/src/main/java/org/photonvision/vision/processes/PipelineManager.java @@ -237,7 +237,6 @@ public class PipelineManager { public void removePipeline(int index) { if (index < 0) { - logger.debug("Could not remove preset pipes!"); return; } // TODO should we block/lock on a mutex? diff --git a/photon-server/src/main/java/org/photonvision/vision/processes/VisionModule.java b/photon-server/src/main/java/org/photonvision/vision/processes/VisionModule.java index 7fe9e10c6..9f2b8cbfc 100644 --- a/photon-server/src/main/java/org/photonvision/vision/processes/VisionModule.java +++ b/photon-server/src/main/java/org/photonvision/vision/processes/VisionModule.java @@ -132,15 +132,8 @@ public class VisionModule { if (event instanceof IncomingWebSocketEvent) { var wsEvent = (IncomingWebSocketEvent) event; - // TODO: remove? - if (wsEvent.propertyName.equals("save")) { - logger.debug("UI-based saving deprecated, ignoring"); - // saveAndBroadcast(); - return; - } - if (wsEvent.cameraIndex != null && wsEvent.cameraIndex == moduleIndex) { - logger.debug("Got PSC event - propName: " + wsEvent.propertyName); + logger.trace("Got PSC event - propName: " + wsEvent.propertyName); var propName = wsEvent.propertyName; var newPropValue = wsEvent.data; @@ -182,7 +175,6 @@ public class VisionModule { logger.debug("Skipping pipeline change, index " + index + " already active"); return; } - logger.debug("Setting pipeline index to " + index); setPipeline(index); saveAndBroadcastAll(); return; diff --git a/photon-server/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java b/photon-server/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java index 28b9f97d7..603e17533 100644 --- a/photon-server/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java +++ b/photon-server/src/main/java/org/photonvision/vision/processes/VisionSourceManager.java @@ -64,27 +64,21 @@ public class VisionSourceManager { private static List filterAllowedDevices(List allDevices) { List filteredDevices = new ArrayList<>(); for (var device : allDevices) { + var deviceInfoStr = + "\"" + + device.name + + "\" at \"" + + device.path + + "\" with USB ID \"" + + device.vendorId + + ":" + + device.productId + + "\""; if (deviceBlacklist.contains(device.name)) { - logger.info( - "Skipping blacklisted device - \"" - + device.name - + "\" at \"" - + device.path - + "\" with VID/PID: " - + device.vendorId - + ":" - + device.productId); + logger.info("Skipping blacklisted device - " + deviceInfoStr); } else { filteredDevices.add(device); - logger.info( - "Adding local video device - \"" - + device.name - + "\" at \"" - + device.path - + "\" with VID/PID: " - + device.vendorId - + ":" - + device.productId); + logger.info("Adding local video device - " + deviceInfoStr); } } return filteredDevices; @@ -110,7 +104,7 @@ public class VisionSourceManager { if (StringUtils.isNumeric(config.path)) { // match by index var index = Integer.parseInt(config.path); - logger.trace( + logger.debug( "Trying to find a match for loaded camera " + config.baseName + " with index " + index); cameraInfo = detectedCameraList.stream() @@ -119,7 +113,7 @@ public class VisionSourceManager { .orElse(null); } else { // matching by path - logger.trace( + logger.debug( "Trying to find a match for loaded camera " + config.baseName + " with path " @@ -134,14 +128,14 @@ public class VisionSourceManager { // If we actually matched a camera to a config, remove that camera from the list and add it to // the output if (cameraInfo != null) { - logger.trace("Matched the config for " + config.baseName + " to a physical camera!"); + logger.debug("Matched the config for " + config.baseName + " to a physical camera!"); detectedCameraList.remove(cameraInfo); cameraConfigurations.add(config); } } // If we have any unmatched cameras left, create a new CameraConfiguration for them here. - logger.trace( + logger.debug( "After matching loaded configs " + detectedCameraList.size() + " cameras were unmatched."); for (UsbCameraInfo info : detectedCameraList) { // create new camera config for all new cameras @@ -162,7 +156,7 @@ public class VisionSourceManager { cameraConfigurations.add(configuration); } - logger.trace("Matched or created " + cameraConfigurations.size() + " camera configs!"); + logger.debug("Matched or created " + cameraConfigurations.size() + " camera configs!"); return cameraConfigurations; }