microsoft / MixedReality-WorldLockingTools-Unity

Unity tools to provide a stable coordinate system anchored to the physical world.
https://microsoft.github.io/MixedReality-WorldLockingTools-Unity/README.html
MIT License
188 stars 45 forks source link

[ISV] FWE diagnostics continue after toggling off #235

Closed fast-slow-still closed 2 years ago

fast-slow-still commented 2 years ago

With application running, turn diagnostics on from script.

Logging starts fine.

Turn diagnostics off from script. Logging seems to continue.

srinjoym commented 2 years ago

Method we're using to disable/enable logging:

    public void SetWLTLogging(bool enabled)
    {
        var wltMgr = WorldLockingManager.GetInstance();

        DiagnosticsSettings diag = new DiagnosticsSettings()
        {
            UseDefaults = !enabled,
            Enabled = true,
            StorageSubdirectory = AppServices.AlignmentValidationService.DiagnosticsPath,
            StorageFileTemplate = "FrozenWorld-[Machine]-[Timestamp].hkfw",
            MaxKilobytesPerFile = 2048,
            MaxNumberOfFiles = 64
        };

        wltMgr.DiagnosticsSettings = diag;

        if (!enabled)
        {
            // Upload hkfw files to storage
            AppServices.AlignmentValidationService.UploadDiagnostics();
        }
    }

Repro steps (on HL2):

  1. We're starting with logging turned off on the World Locking Context: image
  2. Use method above to enable logging
  3. WLT starts logging to diagnostics folder (keep running for at least a minute)
  4. Use method above to disable logging
  5. Bug: WLT keeps writing .hkfw files to diagnostics folder
  6. Bug: WLT keeps writing .hkfw files even after repeated enable/disables using method above

If this doesn't repro I would recommend keeping it enabled for longer and enabling/disabling a few times

fast-slow-still commented 2 years ago

Hi @srinjoym , please try the following. Note the slight change to the diagnostics settings UseDefaults and Enabled:

        public void SetWLTLogging(bool enabled)
        {
            var wltMgr = WorldLockingManager.GetInstance();

            DiagnosticsSettings diag = new DiagnosticsSettings()
            {
                UseDefaults = false,
                Enabled = enabled,
                StorageSubdirectory = AppServices.AlignmentValidationService.DiagnosticsPath,
                StorageFileTemplate = "FrozenWorld-[Machine]-[Timestamp].hkfw",
                MaxKilobytesPerFile = 2048,
                MaxNumberOfFiles = 64
            };

            wltMgr.DiagnosticsSettings = diag;

            if (!enabled)
            {
                // Upload hkfw files to storage
                AppServices.AlignmentValidationService.UploadDiagnostics();
            }
        }
fast-slow-still commented 2 years ago

I'm guessing that lack of response means this is no longer an issue. Closing. Feel free to re-open if the problem persists after making the suggested change to your code.

srinjoym commented 2 years ago

@fast-slow-still I don't have access to re-open this issue, pinging for visibility

Sorry for the delay on the response, finally got a chance to test this solution today and it seems to have fixed it! Great catch on the incorrect enabled state.

It looks like setting UseDefaults to true doesn't disable logging. I would open the issue again as this behavior still seems like a bug to me. If it's by design, it would be great to add some docs on how to enable/disable diagnostics with a note on this

fast-slow-still commented 2 years ago

The default is to disable logging. Setting UseDefaults to true disables logging. If you click on UseDefaults in the Unity Inspector, you can see this.

fast-slow-still commented 2 years ago

By the way, this is already documented. The Diagnostics settings are documented here: https://microsoft.github.io/MixedReality-WorldLockingTools-Unity/DocGen/Documentation/HowTos/WorldLockingContext.html#diagnostics-settings

And there is a special section explaining the UseDefaults here: https://microsoft.github.io/MixedReality-WorldLockingTools-Unity/DocGen/Documentation/HowTos/WorldLockingContext.html#default-settings

srinjoym commented 2 years ago

Ah got it, thanks for the pointer on the docs! It seems a bit confusing that the defaults can change to the current value, but I think that's a separate discussion. Should be good to keep this issue closed