Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapts tv_grab_pt_vodafone to new API version #249

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

nsenica
Copy link
Contributor

@nsenica nsenica commented Dec 23, 2024

Thanks for taking the time to open a Pull Request (PR). Please take a moment to review our open/closed PRs above, in case a similar contribution has already been offered.

If you are opening a new Pull Request, please give it a descriptive title and fill out the blanks below, providing as much information as possible.

Pull Requests should be made against our master branch, and rebased if necessary.

What type of Pull Request is this?

  • adds new functionality
  • fixes/improves existing functionality

Does this PR close any currently open issues?

closes: #235

Please explain what this PR does

Refactors PT Vodafone grabber to work with the new available API.

Any other information?

Due to contraints the available channels need to be on a separate file as there isn't any feasible way to fetch it programmatically without sharing personal details.

Where have you tested these changes?

Operating System: OSX 12.7.6

Perl Version: v5.30.3

@nsenica
Copy link
Contributor Author

nsenica commented Dec 23, 2024

cc: @higuita

@higuita
Copy link
Contributor

higuita commented Dec 28, 2024

Not sure why some checks failed, but my tests show that this is working fine

@knowledgejunkie
Copy link
Contributor

Not sure why some checks failed, but my tests show that this is working fine

Looks like transient network and/or container issues, rather than issues with the XMLTV code.

@higuita
Copy link
Contributor

higuita commented Dec 30, 2024

So right now the channel.list needs to be located in the same place as the grabber... but if we execute the grabber with full path from other directory, it fails to find the channel.list

so we have 2 options:

  1. detect the grabber path and add that path to the channel.list
use File::Basename;
my $dirname = dirname(__FILE__);
my $input_file = $dirname . '/channel.list';
  1. add a new option for changing the channel.list location

So what you think would be the best solution

@honir
Copy link
Contributor

honir commented Dec 31, 2024

Some of the other grabbers use $ENV variables for this. e.g.

sub get_default_dir {
	my $winhome = $ENV{HOMEDRIVE} . $ENV{HOMEPATH}
			if defined( $ENV{HOMEDRIVE} ) and defined( $ENV{HOMEPATH} );
	my $home = $ENV{HOME} || $winhome || ".";
	return $home;
}
sub get_default_cachedir {
	return get_default_dir() . "/.xmltv/cache";
}

Maybe something like that?

@nsenica
Copy link
Contributor Author

nsenica commented Dec 31, 2024

@higuita decided to go with your solution, seems to be the cleanest one. nice catch this one :)

@honir your proposal is meant for the cache dir located in the home path, this is file is not generated from within xmltv but bundled with it, otherwise this would have been a better solution.

thanks for all input.

@honir
Copy link
Contributor

honir commented Dec 31, 2024

your proposal is meant for the cache dir located in the home path

Not necessarily -- that was only an example -- I use it also for bundled files (in ~/.xmltv )

And you can, of course, alter the ENV variables to anything you like :-)

@garybuhrmaster
Copy link
Contributor

Not sure why some checks failed, but my tests show that this is working fine

Looks like transient network and/or container issues, rather than issues with the XMLTV code.

There were various issues with the github actions workflow that had nothing to do with this particular PR (the build workflow was failing in some steps for everyone, even the weekly scheduled tests).

I believe the workflows now work properly, so future PRs (or existing ones rebased) should run without build workflow errors, until the next time some distro changes something important (and there is always a next time).

@nsenica
Copy link
Contributor Author

nsenica commented Jan 14, 2025

@garybuhrmaster thanks, can the workflows be re-triggered ? I couldn't find ways to do it on my own.

@garybuhrmaster
Copy link
Contributor

@garybuhrmaster thanks, can the workflows be re-triggered ? I couldn't find ways to do it on my own.

If you rebase your branch to the current master and push it should run with the new workflow recipes.

@nsenica
Copy link
Contributor Author

nsenica commented Jan 14, 2025

ready to be reviewed :) thanks

Copy link
Contributor

@higuita higuita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have been testing, all seems to work
OK to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tv_grab_pt_vodafone stopped working
5 participants