philippe44 / AirConnect

Use AirPlay to stream to UPnP/Sonos & Chromecast devices
Other
3.5k stars 216 forks source link

Fix '-o' option when 'ModelNumber' is NULL #289

Closed pwt closed 3 years ago

pwt commented 3 years ago

The fix included in v0.2.28.1 needs a small correction. If the -o option is being used, and ModelNumber is NULL, the speaker should be excluded.

Only speaker model numbers that match the specific '-o' list should be included.

pwt commented 3 years ago

This can just be squash-merged. The intermediate commits don't add any value.

philippe44 commented 3 years ago

Are you sure? I user has given a list of models to be included but a discovered device returns NULL for this attribute, I'd rather be more liberal and include it (in other word, empty matches everything, cannot be excluded and is always included - you'd have to explicitly disable it in a config file)

If I don't do that, then empty will be excluded if you have -o and have no way to be managed. If I adopt the "liberal" approach, you can still tweak and get what you want (exclusion) in a config file.

pwt commented 3 years ago

Yes, I'm sure.

This currently breaks the use case where one wants only to include Sonos speakers that don't support AirPlay 2. Even though I'm specifying only the required Sonos model numbers, I'm seeing other devices (e.g., SONY TVs) added to the list of renderers, because these return NULL model numbers.

Consider the use case of 'plug and play' environments where a user just plugs a preconfigured device into their network, and it provides AirPlay for their older Sonos speakers. Users of such devices wouldn't know where to start with editing XML files.

The other exclusion options are quite liberal in what they include, if that's what's wanted (which was why I originally wanted the '-o' option).

philippe44 commented 3 years ago

I understand your point although I don't fully agree but I've pushed the following compromise: device that return NULL model number will not be included when -o is set unless \<NULL> is added to the -o list (needs to be properly quoted)

pwt commented 3 years ago

OK, sounds like a good compromise. The point of '-o' really is to be specific about what devices are included; this satisfies that requirement very neatly.

Thanks for your help.

philippe44 commented 3 years ago

Thanks for the support you give to others here!

BTW, is there a few key features you'd like to see from AirConnect (aside from AirPlay2 :-))

pwt commented 3 years ago

Thanks for the support you give to others here! BTW, is there a few key features you'd like to see from AirConnect (aside from AirPlay2 :-))

Hi @philippe44, and sorry for the delayed response.

I get lots of value from AirConnect, so I'm happy to help where I can. Btw, feel free to ask me to help, if you think I can.

I also really appreciate your attentiveness to the occasional issues that emerge, but overall AirConnect is really, really solid. I have it running, always on, in several 'hands-off' environments, and generally it just keeps running perfectly without attention, whatever happens on the network. My experience is limited to using AirConnect with Sonos, which is why there are quite a few questions where I don't have the expertise to help.

Note that I maintain a fairly popular AirConnect Docker image for Raspberry Pi (and other ARM) devices, optimised for Sonos, at: https://github.com/pwt/docker-airconnect-arm. I provide your airupnp-arm binary, plus a lightly modified one that I build, which omits some of the FLUSH commands, meaning that AirConnect + Sonos + iTunes/Apple Music works properly when skipping tracks, etc. Users can choose which binary to run.

In terms of key features: I don't see how AirPlay 2 could be implemented in a bridge device, even if one had the engineering specs, because of the synchronisation challenges.

Additions that would be nice, if they are practical, would be additional back-channel control over next/previous track, and possibly stop/restart -- i.e., control these from the player device (or its controller app) in the same way that volume can controlled from the player. As I say, I don't know if this is even possible within the protocol, let alone whether it could be implemented in a bridge device.

Thanks again for the great work on AirConnect.