Opened 11 years ago
Closed 11 years ago
#3138 closed enhancement (fixed)
appendchaninfo is defined for webif and monitor module, but is also used for logging
Reported by: | malakudi | Owned by: | |
---|---|---|---|
Priority: | minor | Component: | General |
Severity: | low | Keywords: | appendchaninfo webif monitor log |
Cc: | Sensitive: | no |
Description
Reason for enhancement
appendchaninfo is defined for webif and monitor module, but is also used for logging. So, if you don't build monitor or webif module, you can't use the option. It should be allowed to be configured globally. Maybe then remove option from webif or monitor module. Manpages need to be modified too. Attached patch makes the option only global, but feel free to adapt it.
Possible impacts on other features
None as I can tell.
Attachments (1)
Change History (19)
by , 11 years ago
Attachment: | appendchaninfo.patch added |
---|
comment:1 by , 11 years ago
That makes sense (we already have users complaining about appendchaninfo in both sections). Historically the option was in the monitor section. But when new config parser was introduced the people started complaining (rightfully) that when monitor module is disabled appendchaninfo was no longer recognized. So then I added the same option is webif and it used the same variable internally.
I like your solution even better.
Lets see what others have to say about this.
comment:2 by , 11 years ago
Unless there is requirement for different behaviour in logging/webif/monitor, then it makes sense to only be global. If different behaviour is expected, then we have to change the option completely (mon_appendchaninfo, webif_appendchaninfo, log_appendchaninfo etc).
comment:4 by , 11 years ago
NO GLOBAL option!!!!
Please let people select themselves: webif, monitor and log.
Eventually webif/monitor as combined option, because when you want appendchaninfo in webif then it is assumed that is also so in monitor.
So:
Appendchaninfo webif: ON/OFF
Appednchaninfo monitor: ON/OFF
Appendchaninfo logfile: ON/OFF
or
Appendchaninfo webif/monitor: ON/OFF
Appendchaninfo logfile: ON/OFF
comment:6 by , 11 years ago
global. nobody will use different options in webif, monitor and logfile. turning it on and off is enough. to make the output more readable it is better to use the ecmfmt variable.
comment:7 by , 11 years ago
@ni_hao: Please provide situation where you want channel info on monitor/webif but you don't want it on logfile. When is this really needed?
follow-up: 10 comment:8 by , 11 years ago
If needed, I can suggest bitwise configuration.
global value
appendchaninfo=1 logfile only
appendchaninfo=2 webif only
appendchaninfo=4 monitor only
Values of 3,5,6,7 (log+web,log+monitor,web+monitor,all)
I can provide such patch if it is needed.
comment:9 by , 11 years ago
@malakudi: when I am abroard and have to dial-in, then I want to have a quick look (webif) and I prefer not to see appendchaninfo. I do want to have it in logfile, so that I can look to it later (when I am home again) if necessary. However, it is for me not a big issue. I can live with global.
comment:10 by , 11 years ago
Replying to malakudi:
If needed, I can suggest bitwise configuration.
global value
appendchaninfo=1 logfile only
appendchaninfo=2 webif only
appendchaninfo=4 monitor only
Values of 3,5,6,7 (log+web,log+monitor,web+monitor,all)
I can provide such patch if it is needed.
Nice idea :)
comment:12 by , 11 years ago
There's no sense for separate configuration in my eyes (and I also doubt if it's necessary to config this at all):
- First usage for setting is if the channelname appears in the log (this automatically implies monitor interface as this interface simply takes the log)
- Second usage is on status page of WebIf where probably everyone wants to see the channelname...
comment:13 by , 11 years ago
In my opinion, this option can be turned on by default with no user configurable setting at all. As the discussion on reducing configuration items is already ongoing this is a perfect example for me.
Just imagine that it had been turned on by default since the beginning, then now nobody would really start a discussion on it. The chan info would simply be there and the users would be happy with the fact.
Whenever you create an user configurable option you will find someone who is absolutely convinced that he needs it or otherwise the whole application will cease to exists and endanger his immortal soul. Even if it is only user out of 1000.
comment:14 by , 11 years ago
After looking at the code, I agree with Admin. monitor justs takes log from logfile, so it can't be set differently without major changes in the code. Also, current implementation works like global, even if it is defined in two places. So I think my patch is OK, just make it global. Of course we could also remove the option and make it the default. If someone doesn't want to have channel info, just don't use oscam.srvid file.
comment:15 by , 11 years ago
oscam have way too many configuration options, I vote to just kill appendchaninfo. malakudi is right, if someone doesn't want channel names its easier to just remove oscam.srvid file.
Currently you have to fill oscam.srvid and than start to wonder why you don't have channel names until you found "appendchaninfo" and enable it. It makes no sense.
comment:18 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I think r8546 closes this ticket.
patch for appendchaninfo config to be applied globally