[pvr] Enable channel name updates from the client #4412
Conversation
|
looks good. |
|
@opdenkamp @jmarshallnz we should get this in for Gotham |
|
aren't we in the "crash&burn only" phase already? |
|
It's pretty annoying to have to reset the database every time you rename a channel, though I understand if you don't want this for Gotham. It's a pretty isolated change though. |
Yes |
|
Deferred to G+1 |
|
too bad ;) |
|
how often do you guys rename your TV channels so that you consider this fix as important? There are far more "important" issues to fix in PVR :) |
|
I don't, it's the providers that do. |
|
yes and nobody knows that your name changes in your tv backend aren't reflected to xbmc. for example on christmas time sky cinema hd is sky christmas hd, but i hope until next christmas we have merged this ;) |
|
@jmarshallnz it's fine with a separate column, as we don't have much more info which could be updated in xbmc. @Jalle19 do not forget to check my comment. |
|
@xhaggi rebased and added the last commit |
|
@jmarshallnz or @opdenkamp should we squash all into one commit? |
|
@xhaggi fixed |
|
I don't have a preference, but it seems the first commit is separate, and the last 5 could be combined into 1 if you wanted. |
|
yeah combining those in two commits would be nice |
|
Okay, I'll squash the last four commits into one. Just need to figure out a good commit message. |
|
use [pvr] enable channel name updates from the client as you've done in the title of the PR ;) |
|
could someone trigger jenkins .. i've lost my magic. |
|
jenkins build this please |
|
@Jalle19 needs a rebase please .. rebabe .. i'm on my tablet :) |
|
@Jalle19 is there not a simpler solution to this... Why not change the PVRChannel usage so that the PVR clients NEVER set the main name, they only ever set the client one. I believe that's a few lines change in the constructor (could be wrong). Then change the ChannelName() access routine to return m_strChannelName if not empty, else return the client name. This is actually how channel names work in TVH, if user sets a channel name, use that, if not use the underlying service name. Much less changes than you have here to achieve almost the same thing. The only diff, could be seen as advantage or disadvantage depending on your take, is that if the user sets the channel name to "", then it will revert to the client version. |
|
What do you other guys think? We'd need another string column for the client name, but I agree that the logic would a bit simpler. |
|
haven't had time to review the PR yet, but quick comment here while waiting for an upload to finish: won't work, because the client may be offline. |
|
this only makes sense if |
|
@Jalle19 personally I wouldn't bother, if you can, just persist the user defined name to the database. If the client is offline, who cares what the channel name is, it won't be useful anyway. |
|
the UI does care. we need to store the name in our local db |
|
@opdenkamp if the client is offline the whole channel will be removed anyway. |
|
But really I'm not that bothered what the solution is, but I never set channel names in XBMC, so its crazy that if my channels change name (ro something odd happens) that the only way to get things updated is to a) manually change all the channel names to match or b) reset the DB (i.e. the normal solution to this problem) |
|
no it won't. and i'm going to continue with my work now and review and comment on it later :) |
|
@adamsutton with this PR you get channel updates. |
|
@opdenkamp sounds like another issue then, what's the logic behind keeping channels when the client is off line? (Power saving backends?) @xhaggi sure, I completely get that, but just seemed like an overly complicated approach, I'm happy with either as long as it solves the problem. @Jalle19 and I were discussing this as I noted it while looking at something else and he asked me to put a comment on here so he didn't forget. Job done. |
|
e.g. no wifi connection |
|
@opdenkamp that's surely not the same? A transient failure is a different issue. |
|
A transient failure will not cause all channels to be removed, unless there is something very wrong in the addon? The data should be persisted internally until a reconnection is made, no? |
|
another possible solution would be storing the client channel name too and leave the channel name empty except the user change it. this way we could do it like @adamsutton proposed. |
|
@xhaggi I think that's what he meant from the beginning. |
|
@xhaggi @Jalle19 that's exactly what I meant, I just wasn't aware of the issue regarding what is/isn't saved to the DB. Personally I wouldn't bother saving client names, if the client is off-line, it can't be used. But I don't pretend to understand the fully ramifications of such a decision and defer that to those that do. |
|
for internal purposes, i'd prefer to store the name of the underlying client channel. in that way we do it now or in that way @adamsutton proposed. |
|
@xhaggi rebased, had to bump the database version to 24. I'm fine with the current solution for now, no point in reworking it already. |
|
thanks @Jalle19 jenkins build this please |
|
jenkins build this please (he screwed up) |
[pvr] Enable channel name updates from the client
Fixes http://trac.xbmc.org/ticket/14930
This fixes the above mentioned ticket (that channel name changes on the backend aren't ever reflected in XBMC unless the PVR database is reset). Now the name is updated from the client automagically except when it has been changed through the channel manager. If it is later changed to an empty string it will again be updated by the backend the next time XBMC is restarted.
@xhaggi and @opdenkamp please review.
If in the future we find that we need another "isUserSet" column we should consider changing them all into a single bitflag column which indicates what has been changed. For now though I think this is a good enough solution.