HeartbeatChannel with ILogger raises NPE

[RobertK66]

In version 1.1.1 the new Logging in the HeartbeatChannel breaks with my Client application.

That's because I provide a ILogger Factory and therefore there is no generic ILogger available for this class. ( I think the pattern here would be to get a ILogger<MediaChannel> from DI.

And also your code should always log with _logger?.Log... !!!

[Tapanila]

That is A very good point. Would you be willing to open a pull request to fix this? I will take a look at on weekend otherwise

[RobertK66]

ok, I will try to fix it this evening ....

[RobertK66]

This was harder than I expected 😮‍💨! The crucial line is:

        serviceCollection.AddSingleton<ILoggerFactory>(loggerFactory);
        serviceCollection.AddSingleton(typeof(ILogger<>), typeof(Logger<>)); // This is to allow ILogger<T> to work in DI !!!!

inspired from here: https://stackoverflow.com/questions/31751437/how-is-iloggert-resolved-via-di

I am not ready with a clean pull request yet. But the Fix is in https://github.com/RobertK66/SharpCaster/commit/93d81f0ee773f268519f7ae7807da6ec4c7af0ed for 'preview'.

What I would like to add now is a Cleanup with following:

• allways use ILoggerFactory and not a direct Logger instance
• inject a Logger in all Channels
• Allow logging granularity somehow to get the Low Level Sendmessages filtered (e.g. no Ping/PONG messages because they do log at 'trace' and not 'debug' level or smth. like this ....

I will need some time to prepare this....

[RobertK66]

I also found another problem with the heartbeat timer:

10:11:31 ChromecastChannel Trace SENT    : d8bf342e-0b57-4dc6-ae5e-17ab63e66ba0: {"mediaSessionId":7,"requestId":1830495792,"type":"QUEUE_NEXT"}
10:11:35 HeartbeatChannel Debug Heartbeat timeout
10:11:35 HeartbeatChannel Debug Stopped heartbeat timeout timer

It should reset if other messages are received! The above log was a faill during my QueueTests for Next Prev -> the messages sent around do prevent a Ping to be sent by the device!

[Tapanila]

That's interesting. My understanding is that the ping - pong messaging should be happening all the time.

Does it look like that you don't need to do the ping - pong messaging if there has been some other messaging? I will need do more testing on this but if that's the case I need to change the logic a bit

[Tapanila]

If we want to do the logger in all channels (which probably makes a lot of sense) I think the logger could be forced from the base class.

[Tapanila]

You were totally right about the heartbeat timer. Wrote a unit test for it and fixed the bug. I'm totally fine with changing the levels of any messages didn't really put much effort on thinking those