[mythtv] [mythtv-commits] mythtv/master commit: 45a3bfcd4 by Robert McNamara (rmcnamara)
Michael T. Dean
mtdean at thirdcontact.com
Tue Feb 7 17:44:33 UTC 2012
On 02/07/2012 08:12 AM, David Blain wrote:
>> Author: Robert McNamara<rmcnamara at mythtv.org> Change Date:
>> 2012-02-06T21:35:43-08:00
>> Push Date: 2012/02/06 21:39:00 -0800
>> Repository: mythtv
>> Branch: master
>> New Revision: 45a3bfcd4cde5332222056f99b740c163cfe92e2
>> Changeset: https://github.com/MythTV/mythtv/commit/45a3bfcd4
>>
>> Log:
>>
>> Convert to string output for rec/searchtype, dupmethod/in in Services.
>>
>> Removes use of internal enums and uses translated strings instead.
>>
>> Apologies to anyone using the recrule services for the changes, but it was
>> probably just me given the bug I just fixed preventing them from working
>> right.
> Is there a reason you couldn't expose the enum from the service. I kind of
> wanted to keep all datacontracts how we'd want to use them internally.
>
> If the enum is used instead of string or int as the return type, with a
> little more use of Qt macros, it should serialize the enum correctly (see
> recording.h).
FWIW, I think external, 3rd-party clients should not be interpreting
data based on raw, internally-defined values. We have far too many 3rd
party clients that get broken spectacularly when we change/add/remove
values for such things (even clients using bindings have been affected
by past changes). And, since our 3rd-party clients have a tendency to
not actually care about version changes (and fake protocol/version
compatibility), this can result in not only those clients failing to
work, but actually doing things to break MythTV (at minimum sending
wrong values so that, for example, recording rules are set up
incorrectly or, worse, actually corrupting data, which may cause more
extreme failures of mythbackend/mythfrontend).
My preference for external API use is to present the data such that
clients don't need to know that (currently) 0 means HDTV, 1 means
WIDESCREEN, and 2 means AVC for program's videoprop, and for
recordedprogram's videoprop, there are additional values, 3 = 720 and 4
= 1080 and (as of only very recently) 5 = DAMAGED. It would also be
nice if clients didn't need to know that, for dupin values,
1=kDupsInRecorded, 2=kDupsInOldRecorded, 15=kDupsInAll, and
16=kDupsNewEpi (and other values are invalid--though in 0.24, we also
had meanings for 32, 64, and 128). Since we're constantly changing our
internal values, I don't want clients trying to interpret those values.
I understand the desire to use internal values so the services API (or
at least datacontracts) can be used for internal code, too, but we have
to do something so that clients don't have to know these
constantly-changing MythTV internals. I think even if we use "external"
values that are specific to the API--and map them to internal
values--we'll have the same issue with changes over time. This approach
could minimize the damage changes do over time, but would result in
cluttering the external API values.
My current thinking is that the best approach would be making it so that
clients aren't interpreting data at all--but rather displaying it and
allowing user interaction with it. At minimum, we need to provide a
translation between "raw values" and their meanings. I've created a
LabelValue datacontract that I'm using with the logging code (which I
will be pushing soon) to provide label/value pairs for the UI as well as
"selected" and "active" flags (i.e. for use in drop down lists or
similar). We could use this approach to allow providing internal values
and a user-friendly label for those values. In the context of setting
up recording rules, it even makes sense to add a description that allows
providing an explanation that clients could display for users. And this
approach may actually make translation easier--as it would allow
clients/users to see the user-friendly label, while providing back the
currently-mapped-to-that-label internal value.
If others agree with this approach--and if no one else has a better
idea--I can convert the scheduling code to use that approach for you,
Robert. I'm pretty sure this change came out of comments I made in IRC
about not using internal values in the external API (and mentioned using
the "external" values approach since I hadn't come up with a better
approach at the time--which is what you've done here), so since you've
already changed it once for me, it makes sense for me to change it this
second time.
Thoughts/suggestions, David (or anyone else)?
Mike
More information about the mythtv-dev
mailing list