PDA

View Full Version : Fwd: Re: [slim] SlimServer 6.0b1 Cannot connect



kdf
2005-03-09, 23:54
I've forwarded a post from the discussion list, in case anyone wants to add any
feedback. I've attached a diff of what I think are the changes Bill is
suggesting here.

-kdf

----- Forwarded message from Bill Moseley <moseley (AT) hank (DOT) org> -----
Date: Wed, 9 Mar 2005 22:06:50 -0800
From: Bill Moseley <moseley (AT) hank (DOT) org>
Reply-To: Slim Devices Discussion <discuss (AT) lists (DOT) slimdevices.com>
Subject: Re: [slim] SlimServer 6.0b1 Cannot connect
To: Slim Devices Discussion <discuss (AT) lists (DOT) slimdevices.com>

Not sure if anyone with cvs access is listening, but the problem is
that the client init code (which loads the default variable) is not
being called. It's not very clear what is suppose to happen. Here's
what is happening, though:

In Slim::Web::HTTP::processURL() there's this:

if ($paddr) {
$client = Slim::Player::HTTP->new($address, $paddr, $httpClient);
$client->init();
}

The question is which init() is that suppose to be??

So, in Slim::Player::HTTP::new() $client is created like this:

my $client = Slim::Player::Client->new($id, $paddr);

And indeed in Slim::Player::Client there's an init() *function* that
sets the default variables. Note that this is not a method call, and
it's also never being called. Here's that init function:

sub init {
my $client = shift;
# make sure any preferences unique to this client may not have set are set to
the default
Slim::Utils::Prefs::initClientPrefs($client,$defau ltPrefs);
}


So, at this point calling $client->init would initialize the
default client variables (if it was a method call).

But, if you look back at Slim::Player::HTTP::new()
$client is re-blessed. It's blessed into the Slim::Player::HTTP
class from the Slim::Player::Client class.


sub new {
my ($class, $id, $paddr, $tcpsock) = @_;

my $client = Slim::Player::Client->new($id, $paddr);

$client->streamingsocket($tcpsock);

bless $client, $class; # re-bless $client.

return $client;
}

So when $client->init() is called (up there in processURL) it's
not running the init() method that would initialize the client
variables.

I cannot guess what the programmer intended. Perhaps
Slim::Player::HTTP::init() should be modified like:

sub init {
my $client = shift;
$client->SUPER::init(); # add this line
$client->startup();
}

and then Slim::Player::Client::init() should be made into a method call.
Or, maybe the programmer intended to just call init($client) in
Slim::Player::Client::new(). That I cannot guess.

And that doesn't even fix it, because the missing parameter
"lameQuality" is not defined in $Slim::Player::Client::defaultPrefs.

Was "lameQuality" intended to be a client parameter or a global param?
Looks like a client variable.

Slim/Player/Source.pm:
my $quality = Slim::Utils::Prefs::clientGet($client,'lameQuality ');

So I'll just assume that it was left out of $Slim::Player::Client::defaultPrefs
by mistake.


Someday...

I think it would help to have a configuration base class with all
defaults, and override methods for specific client classes.
Something like:

# Add "config" method with this clients prefs
$client->init_prefs( $defaultPrefs );

Then later:

$quality = $client->config->lameQuality;

Which would clean things up quite a bit, and catch typos, too.


--
Bill Moseley
moseley (AT) hank (DOT) org

Robert Moser
2005-03-10, 08:40
kdf wrote:
> I've forwarded a post from the discussion list, in case anyone wants to add any
> feedback. I've attached a diff of what I think are the changes Bill is
> suggesting here.

Patch looks good, although you might want to have the init prior to the
startup.

kdf
2005-03-10, 13:55
Quoting Robert Moser <rlmoser (AT) comcast (DOT) net>:

> kdf wrote:
> > I've forwarded a post from the discussion list, in case anyone wants to add
> any
> > feedback. I've attached a diff of what I think are the changes Bill is
> > suggesting here.
>
> Patch looks good, although you might want to have the init prior to the
> startup.

good point. thanks. I merge that in tonight.

-kdf