Mailing List Archive: SpecialRecordingGroups hard coded IDs

SpecialRecordingGroups hard coded IDs

May 1, 2019, 8:54 AM

Post #1 of 7 (1286 views)

I know this isn't a high priority item but I thought I'd bring it up
here, at least for discussion.

My MySQL instance is setup so that the auto-increment values go up by 2
instead of 1, so my recgroups table has IDs that are all odd. I have
always had to adjust recordinginfo.h and the SpecialRecordingGroups to
match my DB values. I was thinking of doing a patch where the instances
where special IDs are used, they are retrieved from the DB instead of
using the hard-coded values. They aren't used in very many places and I
was just going to add some static members to recordinginfo.cpp to
retrieve them, like:

RecordingInfo::GetDefaultRecGroupId, RecordingInfo::GetLiveTvRecGroupId
and RecordingInfo::GetDeletedRecGroupId

So wherever RecordingInfo::kXXXRecGroup is used would be replaced with
the static methods. I think that would solve my problem without being
too much of an intrusive change to the codebase.

Another issue might be with me submitting a decent patch. I'm only
running fixes/29, but I think I'd only be modifying a few files. I've
never contributed to open source before and am not great with git or
creating patches. I might need some help there.
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

Re: SpecialRecordingGroups hard coded IDs [ In reply to ]

stuarta at squashedfrog

May 2, 2019, 7:13 AM

Post #2 of 7 (1279 views)

Permalink

On 01/05/2019 16:54, mythtv wrote:
> I know this isn't a high priority item but I thought I'd bring it up
> here, at least for discussion.
>
> My MySQL instance is setup so that the auto-increment values go up by 2
> instead of 1, so my recgroups table has IDs that are all odd. I have
> always had to adjust recordinginfo.h and the SpecialRecordingGroups to
> match my DB values. I was thinking of doing a patch where the instances
> where special IDs are used, they are retrieved from the DB instead of
> using the hard-coded values. They aren't used in very many places and I
> was just going to add some static members to recordinginfo.cpp to
> retrieve them, like:
>
> RecordingInfo::GetDefaultRecGroupId, RecordingInfo::GetLiveTvRecGroupId
> and RecordingInfo::GetDeletedRecGroupId
>

We already have the function GetRecgroupId, see [1]. Does that not do
what you were thinking of?

[1] -
https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recordinginfo.cpp#L1603-L1617

> So wherever RecordingInfo::kXXXRecGroup is used would be replaced with
> the static methods. I think that would solve my problem without being
> too much of an intrusive change to the codebase.
>
> Another issue might be with me submitting a decent patch. I'm only
> running fixes/29, but I think I'd only be modifying a few files. I've
> never contributed to open source before and am not great with git or
> creating patches. I might need some help there.
> _______________________________________________
> mythtv-dev mailing list
> mythtv-dev@mythtv.org
> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
> http://wiki.mythtv.org/Mailing_List_etiquette
> MythTV Forums: https://forum.mythtv.org

_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

Re: SpecialRecordingGroups hard coded IDs [ In reply to ]

mythtv at mdabbs

May 2, 2019, 7:46 AM

Post #3 of 7 (1279 views)

Permalink

On 5/2/19 9:13 AM, Stuart Auchterlonie wrote:
>
> We already have the function GetRecgroupId, see [1]. Does that not do
> what you were thinking of?
>
> [1] -
> https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recordinginfo.cpp#L1603-L1617
>
That would work too, although I wanted to avoid using the String
versions of the special groups because I thought I saw somewhere in the
code that the String versions were trying to be phased out. So having
the separate methods would avoid still using the string names outside of
the recordinginfo class.

I'm okay either way though. I'll defer to the wisdom of the existing dev
team. If everyone has an aversion to more methods in the class, I could
just call GetRecgoupId and pass the names in.
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

Re: SpecialRecordingGroups hard coded IDs [ In reply to ]

mythtv at mdabbs

May 2, 2019, 9:02 AM

Post #4 of 7 (1277 views)

Permalink

On 5/2/19 9:46 AM, mythtv wrote:
> On 5/2/19 9:13 AM, Stuart Auchterlonie wrote:
>>
>> We already have the function GetRecgroupId, see [1]. Does that not do
>> what you were thinking of?
>>
>> [1] -
>> https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recordinginfo.cpp#L1603-L1617
>>
>>
> That would work too, although I wanted to avoid using the String
> versions of the special groups because I thought I saw somewhere in the
> code that the String versions were trying to be phased out. So having
> the separate methods would avoid still using the string names outside of
> the recordinginfo class.
>
> I'm okay either way though. I'll defer to the wisdom of the existing dev
> team. If everyone has an aversion to more methods in the class, I could
> just call GetRecgoupId and pass the names in.
> _______________________________________________
> mythtv-dev mailing list
> mythtv-dev@mythtv.org
> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
> http://wiki.mythtv.org/Mailing_List_etiquette
> MythTV Forums: https://forum.mythtv.org

Or, maybe better yet, just one method to convert the
SpecialRecordingGroup enum to the actual ID, instead of assuming the
SpecialRecordingGroup *IS* the ID. So something like:

uint RecordingInfo::GetSpecialRecgroupID(SpecialRecordingGroups group)
{
QString names[3] = { QString("Default"), QString("LiveTV"),
QString("Deleted") };

return GetRecgroupID(names[group]);
}

And renumbering the enum to start at 0:

typedef enum {
kDefaultRecGroup = 0,
kLiveTVRecGroup = 1,
kDeletedRecGroup = 2,
} SpecialRecordingGroups;

_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

Re: SpecialRecordingGroups hard coded IDs [ In reply to ]

mythtv at mdabbs

May 3, 2019, 10:48 AM

Post #5 of 7 (1275 views)

Permalink

On 5/2/19 11:02 AM, mythtv wrote:
> On 5/2/19 9:46 AM, mythtv wrote:
>> On 5/2/19 9:13 AM, Stuart Auchterlonie wrote:
>>>
>>> We already have the function GetRecgroupId, see [1]. Does that not do
>>> what you were thinking of?
>>>
>>> [1] -
>>> https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recordinginfo.cpp#L1603-L1617
>>>
>>>
>> That would work too, although I wanted to avoid using the String
>> versions of the special groups because I thought I saw somewhere in
>> the code that the String versions were trying to be phased out. So
>> having the separate methods would avoid still using the string names
>> outside of the recordinginfo class.
>>
>> I'm okay either way though. I'll defer to the wisdom of the existing
>> dev team. If everyone has an aversion to more methods in the class, I
>> could just call GetRecgoupId and pass the names in.
>> _______________________________________________
>> mythtv-dev mailing list
>> mythtv-dev@mythtv.org
>> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
>> http://wiki.mythtv.org/Mailing_List_etiquette
>> MythTV Forums: https://forum.mythtv.org
>
> Or, maybe better yet, just one method to convert the
> SpecialRecordingGroup enum to the actual ID, instead of assuming the
> SpecialRecordingGroup *IS* the ID. So something like:
>
> uint RecordingInfo::GetSpecialRecgroupID(SpecialRecordingGroups group)
> {
>     QString names[3] = { QString("Default"), QString("LiveTV"),
> QString("Deleted") };
>
>     return GetRecgroupID(names[group]);
> }
>
> And renumbering the enum to start at 0:
>
> typedef enum {
>     kDefaultRecGroup     = 0,
>     kLiveTVRecGroup      = 1,
>     kDeletedRecGroup     = 2,
> } SpecialRecordingGroups;
>
> _______________________________________________
> mythtv-dev mailing list
> mythtv-dev@mythtv.org
> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
> http://wiki.mythtv.org/Mailing_List_etiquette
> MythTV Forums: https://forum.mythtv.org

And here's a patch for consideration... (It's against fixes/29, sorry)

Re: SpecialRecordingGroups hard coded IDs [ In reply to ]

pb.mythtv at gmail

Jul 16, 2019, 10:45 AM

Post #6 of 7 (1203 views)

Permalink

On 5/3/19 1:48 PM, mythtv wrote:
> On 5/2/19 11:02 AM, mythtv wrote:
>> On 5/2/19 9:46 AM, mythtv wrote:
>>> On 5/2/19 9:13 AM, Stuart Auchterlonie wrote:
>>>>
>>>> We already have the function GetRecgroupId, see [1]. Does that not do
>>>> what you were thinking of?
>>>>
>>>> [1] -
>>>> https://github.com/MythTV/mythtv/blob/master/mythtv/libs/libmythtv/recordinginfo.cpp#L1603-L1617
>>>>
>>>>
>>> That would work too, although I wanted to avoid using the String
>>> versions of the special groups because I thought I saw somewhere in
>>> the code that the String versions were trying to be phased out. So
>>> having the separate methods would avoid still using the string names
>>> outside of the recordinginfo class.
>>>
>>> I'm okay either way though. I'll defer to the wisdom of the existing
>>> dev team. If everyone has an aversion to more methods in the class,
>>> I could just call GetRecgoupId and pass the names in.
>>> _______________________________________________
>>> mythtv-dev mailing list
>>> mythtv-dev@mythtv.org
>>> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
>>> http://wiki.mythtv.org/Mailing_List_etiquette
>>> MythTV Forums: https://forum.mythtv.org
>>
>> Or, maybe better yet, just one method to convert the
>> SpecialRecordingGroup enum to the actual ID, instead of assuming the
>> SpecialRecordingGroup *IS* the ID. So something like:
>>
>> uint RecordingInfo::GetSpecialRecgroupID(SpecialRecordingGroups group)
>> {
>>      QString names[3] = { QString("Default"), QString("LiveTV"),
>> QString("Deleted") };
>>
>>      return GetRecgroupID(names[group]);
>> }
>>
>> And renumbering the enum to start at 0:
>>
>> typedef enum {
>>      kDefaultRecGroup     = 0,
>>      kLiveTVRecGroup      = 1,
>>      kDeletedRecGroup     = 2,
>> } SpecialRecordingGroups;
>>
>> _______________________________________________
>> mythtv-dev mailing list
>> mythtv-dev@mythtv.org
>> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
>> http://wiki.mythtv.org/Mailing_List_etiquette
>> MythTV Forums: https://forum.mythtv.org
>
> And here's a patch for consideration... (It's against fixes/29, sorry)
>
>
>

I would think that a smaller change would be for the code which
initially adds these special recording groups to the database to hard
code the values it adds as 1,2,3 instead of letting them auto-increment
and hope that 1,2,3 get used. This, however, would not help people who
are already in this situation.

For your situation, or users who already have wrong ids for these
groups, can you just update those groups in your database to 1,2,3 and
run some sql that updates the recorded table entries to use the 1,2,3
values?

A schema update could be done that makes these changes.

I have not looked at the code to see if there are any gotchas with this.

Peter

Re: SpecialRecordingGroups hard coded IDs [ In reply to ]

pb.mythtv at gmail

Jul 16, 2019, 10:47 AM

Post #7 of 7 (1203 views)

Permalink

It seems I have accidentally picked up some old thread and replied to
it. Sorry and please feel free to ignore that.

Peter
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

Mailing List Archive

Mailing List Archive

Attached Files: