On Wed, 2017-01-04 at 11:18 -0800, Juha Kuikka wrote:
> Hi Bastien,
>
> On Thu, Dec 29, 2016 at 3:48 AM, Bastien Nocera <hadess@xxxxxxxxxx>
> wrote:
> > On Wed, 2016-12-28 at 16:32 -0800, Juha Kuikka wrote:
> > > There is a special path for various game controllers where they
> > > connect
> > > to the hid service before bluetoothd knows what they are.
> > >
> > > This patch adds another PID for the Dualshock4 controller. This
> > > new
> > > PID
> > > matches with the model number CUH-ZCT2U.
> > > ---
> > > profiles/input/server.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/profiles/input/server.c b/profiles/input/server.c
> > > index eb3fcf8..3576f2b 100644
> > > --- a/profiles/input/server.c
> > > +++ b/profiles/input/server.c
> > > @@ -135,8 +135,10 @@ static bool dev_is_sixaxis(const bdaddr_t
> > > *src,
> > > const bdaddr_t *dst)
> > > if (vid == 0x054c && pid == 0x0268)
> > > return true;
> > >
> > > - /* DualShock 4 */
> > > - if (vid == 0x054c && pid == 0x05c4)
> > > + /* DualShock 4 CUH-ZCT1U (PID 0x05c4)
> > > + * DualShock 4 CUH-ZCT2U (PID 0x09cc) (slim/pro)
> > > + */
> > > + if (vid == 0x054c && (pid == 0x05c4 || pid == 0x09cc))
> > > return true;
> > >
> > > /* Navigation Controller */
> >
> > Might be nice to have the struct you currently have in the sixaxis
> > plugin in a shared header, so we don't need to open code this
> > function.
> > For example:
> >
> > typedef enum {
> > CABLE_PAIRING_SIXAXIS,
> > CABLE_PAIRING_DS4
> > } CablePairingType;
> >
> > static struct {
> > int vid;
> > int pid;
> > CablePairingType type;
> > } cable_pairing_devices[] = {
> > ...
> > };
> >
>
> I agree, having the pertinent VID/PID in one place only would be a
> good idea. One small issue I see here is that it would unnecessarily
> expose internal information from sixaxis plugin.
Which internal information? I don't think you need to export any
information from the sixaxis plugin to make this work.
> > The sixaxis plugin would know which functions to use from the type,
> > the
> > input/server.c code would loop over the array to see whether it's a
> > cable pairing.
>
> Having a type field instead of the function pointers would make for a
> less clear implementation in sixaxis.c. My previous patch had an
> analogous mechanism for detecting controller type and changing
> behavior accordingly and I got some comments about a function pointer
> approach being preferred.
No, I'm not asking you to replace that. Inside the sixaxis plugin you
used to do:
set_master_bdaddr()
{
if (is_ds4()) {
...
} else if (is_sixaxis()) {
...
}
}
Here, I'm saying that when you're doing your search you should have:
setup_device()
{
if (is_ds4())
device->funcs = ds4_device_funcs;
else if (is_sixaxis()
device->funcs = sixaxis_device_funcs;
}
Or you could also use another array for that:
static struct {
DeviceType type;
SetupFunc setup_func;
PostSetupFunc post_setup_func;
} functions[] = {
CABLE_PAIRING_TYPE_DS4, ds4_setup_func, ...
And set that up in setup_device().
> I saw some patches floating around concerning a generalized cable
> pairing mechanism. Perhaps this change should be part of that?
I don't think we need something more generic at this point.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
