catalinii / minisatip

minisatip is an SATIP server for linux using local DVB-S2, DVB-C, DVB-T or ATSC cards
https://minisatip.org
328 stars 80 forks source link

RFE: Option for disable SSDP messages in the LOG #205

Closed lars18th closed 8 years ago

lars18th commented 8 years ago

Hi,

I suggest to add one option in the command line to silence all SSDP related messages from the log. When I try to debug the connection with a client, several SSDP message are interlaced in the log. These messages are very interesting when you like to debug the entire process, but when you like to see only connection traces, in this case the SSDP are garbagge.

When you work with SAT>IP protocol two different parts exist: DISCOVERY (linked to SSDP) and TRANSPORT (linked to RSTP). So, in order to debug one part, will be useful to 'silence' the other part.

Waht you think?

lars18th commented 8 years ago

Hi,

I commented these LOG lines to silence some SSDP and headers in minisatip.c :

540:    LOG("read RTSP (from handle %d sock_id %d, len: %d, sid %d):\n%s", s->sock,
966:    LOG("Received SSDP packet from %s:%d -> handle %d",
1013:   LOG("ssdp_reply fd: %d -> %s:%d, bootid: %d deviceid: %d http: %s", ssdp,
1214:   LOG("reply -> %d (%s:%d) CL:%d [sock_id %d]:\n%s", s->sock,

I suggest to move these lines to another log level. I feel they are too verbose until level 3 or 4. You agree?

lars18th commented 8 years ago

Hi,

I like to suggest also to incorporate another cosmetic change in logs:

In minisatip.c move the print of TCP messages to Log level 3 or 4. Specifcally these lines:

552:    LOG("read RTSP (from handle %d sock_id %d, len: %d, sid %d):\n%s", s->sock,
553:            s->id, s->rlen, s->sid, s->buf);

797:    LOG("read HTTP from %d sid: %d: %s", s->sock, s->sid, s->buf);

As you can show the "s->buf" prints several lines. I suggest to print them only in superior log levels (typically I see the logs with level 2). At time I'm commenting out them!

You agree?

lars18th commented 8 years ago

Hi @catalinii ,

Simple change for better readability (more easy to isolate relevant info):

Please, at "minisatip.c" replace all "s->buf" logs with "LOGL(3, "%s", s->buf);"

You have already done in some parts, like: http://github.com/catalinii/minisatip/blob/a5c6903886fc1cdeb08cf5e17ca54a57d7448594/minisatip.c#L993

But not in lines like: http://github.com/catalinii/minisatip/blob/a5c6903886fc1cdeb08cf5e17ca54a57d7448594/minisatip.c#L565 http://github.com/catalinii/minisatip/blob/a5c6903886fc1cdeb08cf5e17ca54a57d7448594/minisatip.c#L809

It's a simple "search&replace" task ! Can you do it, please? :)

lars18th commented 8 years ago

Hi @catalinii ,

My pull request 226 (http://github.com/catalinii/minisatip/pull/226) implements this RFE. If you commit, then close this Issue.

I hope you agree. :) Regards!

catalinii commented 8 years ago

I did not accept Option for mute SSDP logs and other cleanups as that is getting more like a general problem... muting a specific component when you are not interested in the logs for it.

And this is not going to get resolved every time by adding a switch.

One of the option would be:

level 1 - http, rtsp level 2 - dvbapi level 4 - ssdp

lars18th commented 8 years ago

Hi @catalinii ,

Thank you for accepting my last patches. I'll continue working on more funtionalities...

However, related to LOG MUTE, let me to explain why I use this:

Obviously, a more complex logging options can be applied. For example, you can use three different log files: each one for one type of data, and use a log level independently for each one. However, I feel this very complex. So, I added only one function (LOGLM: LOG_with_Level_Mutable), for this reason.

Be free to add or not to the main branch. I'll continue using it. Perhaps you can found another similar way to obtain a simular functionality. :)

Thank you for your good work!

catalinii commented 8 years ago

Hi,

My point was that my initial implementation created this problem. But the solution to mute specific parts is not a solution as it is not flexible enough.

Thanks

lars18th commented 8 years ago

Ok. So, perhaps you like this option:

$ ministatip -M 0,3

The parameter "-M" indicates which "modules" you like to mute: 0=general, 1=ssdp, 2=linuxdvb, 3=dvbapi, etc.

It's this more flexible?