Skip to content

Conversation

SergeyGulik
Copy link
Contributor

Hi Patrick,

We discussed it long-long ago, but I have forgotten to add these logging statements.
Please feel free to accept them partially. Also, pay attention to MTConnectHttpServer.cs

            if (_logger != null)
            {
                serverConfig.DebugLogHandler += (string message, string logtaskid, object data)
                    => _logger.LogDebug($"[{logtaskid}] message: {message}, data: {data}");
            }

It definitely served my purpose, but there might be smarter and more elegant way to do it.

Please feel free to accept these changes partially.
Also, pay attention to MTConnectHttpServer.cs
```
if (_logger != null)
            {
                serverConfig.DebugLogHandler += (string message, string logtaskid, object data)
                    => _logger.LogDebug($"[{logtaskid}] message: {message}, data: {data}");
            }
```
It definitely served my purpose, but there might be smarter and more elegant way to do it.
@SergeyGulik
Copy link
Contributor Author

Hi Patrick!

Any news on this PR?
If you are in doubt about merging it, that's ok as well. I will just keep my changes separately and apply them only for debugging.

BTW, what about releasing a new nuget packages? With or without logging it will help me a lot.

Best wishes,
Sergii Gulyk

@PatrickRitchie
Copy link
Contributor

I'm looking at whether or not to pass a logger as you have in this or to just raise an event. I have previously just raised an event for logging and then have the Agent/Adapter/Client application subscribe to the events and use NLog to actually handle logging. I'll have time to review this further next week.

Yes I'm working on a new release this week that will include the other PRs as well as the updates for MTConnect v2.4 and I will publish new Nuget packages as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Reviewing
Development

Successfully merging this pull request may close these issues.

2 participants