Qtopia-interest Archive, April 2008
Musings on the usage of /dev/input/eventXYZ
Message 1 in thread
Hey,
the input device framework in Linux2.6 is getting used for all kind of
notifications. E.g. charging starts, plugging of chargers, headsets, physical
rotation sensors (hinge), ...
A lot of different devices in the Qtopia tree use this framework. The code is
always similar:
- Declare a copy of struct input_event from linux/input.h
- Open a specific device
- Read a keyboard event and do things
Two issues:
- After two copies one could consider creating a class for providing
access to the Linux Input system (what we did in 2003 with Opie)
- The device is opened by number. The number is fairly random and solely
depends on the order of initialisation of drivers and their availability.
Specially when developing, or upgrading a kernel to a new version these
numbers change. So instead of opening these devices by number, open
them by their name ("I want the Neo1973 Keyboard" instead of "7") or
by their struct input_id. From an API point of view by name would be the
best.
comments
z.
PS: Changing device names are a reality...
--
[ signature omitted ]
Message 2 in thread
Holger Freyther wrote:
> Hey,
>
> the input device framework in Linux2.6 is getting used for all kind of
> notifications. E.g. charging starts, plugging of chargers, headsets, physical
> rotation sensors (hinge), ...
>
> A lot of different devices in the Qtopia tree use this framework. The code is
> always similar:
>
> - Declare a copy of struct input_event from linux/input.h
> - Open a specific device
> - Read a keyboard event and do things
>
>
> Two issues:
> - After two copies one could consider creating a class for providing
> access to the Linux Input system (what we did in 2003 with Opie)
yes possibly. depending on what and where and how these are being used. We have kdbplugins,
gfxplugins. As you know, every device is different as well (hardware - buttons, scancodes, etc).
> - The device is opened by number. The number is fairly random and solely
> depends on the order of initialisation of drivers and their availability.
> Specially when developing, or upgrading a kernel to a new version these
> numbers change. So instead of opening these devices by number, open
> them by their name ("I want the Neo1973 Keyboard" instead of "7") or
> by their struct input_id. From an API point of view by name would be the
> best.
>
>
> comments
I don't really see this as a problem, in the normal course of a phone, the kernel is probably the
last thing an integrator/operator is going to really update. and if they do that low level update,
they are probably changing the system as well.
> z.
>
> PS: Changing device names are a reality...
Yes of course. But then the integrator knowns which devices/names it is using.
--
[ signature omitted ]
Message 3 in thread
On Thursday 03 April 2008 05:43:40 Lorn Potter wrote:
> > - The device is opened by number. The number is fairly random and solely
> > depends on the order of initialisation of drivers and their
> > availability. Specially when developing, or upgrading a kernel to a new
> > version these numbers change. So instead of opening these devices by
> > number, open them by their name ("I want the Neo1973 Keyboard" instead of
> > "7") or by their struct input_id. From an API point of view by name would
> > be the best.
> >
> >
> > comments
>
> I don't really see this as a problem, in the normal course of a phone, the
> kernel is probably the last thing an integrator/operator is going to really
> update. and if they do that low level update, they are probably changing
> the system as well.
hehe, how do you think hardware companies/integration is working?
1. you do the hardware
2. you do the kernel
3. you integrate a GUI
4. profit?
You don't have a waterfall model for creating hardware, e.g. with integrating
a GUI you expose kernel bugs, which expose hardware issues, and you get a new
revision of the hardware, a slightly changed kernel... and the result is:
The numbering of devices from userspace is random! They change at development
time! The only constant is the name of the device as the integrator has
the control there.
so instead of open("/dev/input/event7") a API should be provided that allows
the following:
QLinuxInputEventDevice* inputDevice = new QLinuxInputEventDevice("Neo1973
Keyboard");
connect(inputDevice, SIGNAL(keyevent(KeyEvent)), SLOT(handleKeyEvent..
>
> > z.
> >
> > PS: Changing device names are a reality...
>
> Yes of course. But then the integrator knowns which devices/names it is
> using.
Hehe, yeah they know the name, they don't know the device number as this is
fairly random and depends on the initialisation order of the devices at boot
time.
The reason why you don't see this is: You at Trolltech integrate on devices
with a finished Linux port, this is not the reality for people integrating
Qtopia on new hardware.
z
--
[ signature omitted ]
Message 4 in thread
On Thursday 03 April 2008, Holger Freyther wrote:
> On Thursday 03 April 2008 05:43:40 Lorn Potter wrote:
> > > - The device is opened by number. The number is fairly random and
> > > solely depends on the order of initialisation of drivers and their
> > > availability. Specially when developing, or upgrading a kernel to a new
> > > version these numbers change. So instead of opening these devices by
> > > number, open them by their name ("I want the Neo1973 Keyboard" instead
> > > of "7") or by their struct input_id. From an API point of view by name
> > > would be the best.
> > >
> > >
> > > comments
> >
> > I don't really see this as a problem, in the normal course of a phone,
> > the kernel is probably the last thing an integrator/operator is going to
> > really update. and if they do that low level update, they are probably
> > changing the system as well.
>
> hehe, how do you think hardware companies/integration is working?
>
> 1. you do the hardware
> 2. you do the kernel
> 3. you integrate a GUI
> 4. profit?
>
>
> You don't have a waterfall model for creating hardware, e.g. with
> integrating a GUI you expose kernel bugs, which expose hardware issues, and
> you get a new revision of the hardware, a slightly changed kernel... and
> the result is:
>
> The numbering of devices from userspace is random! They change at
> development time!
Then the developers can change the numbers as they change. I don't see the
problem here.
> The only constant is the name of the device as the
> integrator has the control there.
>
> so instead of open("/dev/input/event7") a API should be provided that
> allows the following:
>
> QLinuxInputEventDevice* inputDevice = new QLinuxInputEventDevice("Neo1973
> Keyboard");
> connect(inputDevice, SIGNAL(keyevent(KeyEvent)), SLOT(handleKeyEvent..
Abstracting it at this end might make the input method plugins code look
better. But someone somewhere _still_ have to open /dev/input/eventX
>
> > > z.
> > >
> > > PS: Changing device names are a reality...
> >
> > Yes of course. But then the integrator knowns which devices/names it is
> > using.
>
> Hehe, yeah they know the name, they don't know the device number as this is
> fairly random and depends on the initialisation order of the devices at
> boot time.
Yes, fair enough, but I haven't seen any problems with this. and as far as I
know, it hasn't been an issue for our customers.
> The reason why you don't see this is: You at Trolltech integrate on devices
> with a finished Linux port, this is not the reality for people integrating
> Qtopia on new hardware.
If they are still developing the device, it's easy to change if the device
nodes change as well.
--
[ signature omitted ]
Message 5 in thread
Lorn Potter wrote:
> On Thursday 03 April 2008, Holger Freyther wrote:
>> On Thursday 03 April 2008 05:43:40 Lorn Potter wrote:
>>>> - The device is opened by number. The number is fairly random and
>>>> solely depends on the order of initialisation of drivers and their
>>>> availability. Specially when developing, or upgrading a kernel to a new
>>>> version these numbers change. So instead of opening these devices by
>>>> number, open them by their name ("I want the Neo1973 Keyboard" instead
>>>> of "7") or by their struct input_id. From an API point of view by name
>>>> would be the best.
>>>>
>>>>
>>>> comments
>>> I don't really see this as a problem, in the normal course of a phone,
>>> the kernel is probably the last thing an integrator/operator is going to
>>> really update. and if they do that low level update, they are probably
>>> changing the system as well.
>> hehe, how do you think hardware companies/integration is working?
>>
>> 1. you do the hardware
>> 2. you do the kernel
>> 3. you integrate a GUI
>> 4. profit?
>>
>>
>> You don't have a waterfall model for creating hardware, e.g. with
>> integrating a GUI you expose kernel bugs, which expose hardware issues, and
>> you get a new revision of the hardware, a slightly changed kernel... and
>> the result is:
>>
>> The numbering of devices from userspace is random! They change at
>> development time!
>
> Then the developers can change the numbers as they change. I don't see the
> problem here.
>
>> The only constant is the name of the device as the
>> integrator has the control there.
>>
>> so instead of open("/dev/input/event7") a API should be provided that
>> allows the following:
>>
>> QLinuxInputEventDevice* inputDevice = new QLinuxInputEventDevice("Neo1973
>> Keyboard");
>> connect(inputDevice, SIGNAL(keyevent(KeyEvent)), SLOT(handleKeyEvent..
> Abstracting it at this end might make the input method plugins code look
> better. But someone somewhere _still_ have to open /dev/input/eventX
>
>
>>>> z.
>>>>
>>>> PS: Changing device names are a reality...
>>> Yes of course. But then the integrator knowns which devices/names it is
>>> using.
>> Hehe, yeah they know the name, they don't know the device number as this is
>> fairly random and depends on the initialisation order of the devices at
>> boot time.
>
> Yes, fair enough, but I haven't seen any problems with this. and as far as I
> know, it hasn't been an issue for our customers.
I recently ran into this input numbering issue. With keyboard and mouse
drivers compiled in my Linux kernel, the init sequence was keyboard then
mouse and I used these environment variables:
QWS_KEYBOARD="usb:/dev/input/event0"
QWS_MOUSE_PROTO="Tslib:/dev/input/event1"
However, if I configure Linux so that the keyboard driver is a module
and I modprobe it after boot, the numbering gets switched, and I had to use:
QWS_KEYBOARD="usb:/dev/input/event1"
QWS_MOUSE_PROTO="Tslib:/dev/input/event0"
It's not a huge issue, but it's an annoyance.
,
John
>
>> The reason why you don't see this is: You at Trolltech integrate on devices
>> with a finished Linux port, this is not the reality for people integrating
>> Qtopia on new hardware.
>
> If they are still developing the device, it's easy to change if the device
> nodes change as well.
>
>
--
[ signature omitted ]
Message 6 in thread
On Thursday 03 April 2008 19:42:04 Lorn Potter wrote:
> On Thursday 03 April 2008, Holger Freyther wrote:
> > The numbering of devices from userspace is random! They change at
> > development time!
Right, this means he has to stop his current task, adjust a number, compile
(wait hours for the clean build), test, commit, go back to his original task.
This is wasting
valuable developer time for no obvious reason. And you assume that if someone
says "The devices goes to suspend even if power is plugged in", you
immediately suspect the device numbers have changed... This wasting of time
can be so easily avoided... and this wasting of time is a big annoyance.
>
> Then the developers can change the numbers as they change. I don't see the
> problem here.
How many devices with linux have you built in your career?
> >
> > QLinuxInputEventDevice* inputDevice = new
> > QLinuxInputEventDevice("Neo1973 Keyboard");
> > connect(inputDevice, SIGNAL(keyevent(KeyEvent)), SLOT(handleKeyEvent..
>
> Abstracting it at this end might make the input method plugins code look
> better. But someone somewhere _still_ have to open /dev/input/eventX
>
Yeah, the framework (in this case Qtopia). Just take a look at the available
ioctl's in linux/input.h. Allow the Input Device to be opened by name, or by
the product id, vendor id tuple... This is the power of abstraction. The
product and vendor id, and the name, and capabilities are constants, the
number of the device is not. Do whatever you want with this information, I
for myself consider opening /dev/input/eventFIXED_NUMBER broken.
> > Hehe, yeah they know the name, they don't know the device number as this
> > is fairly random and depends on the initialisation order of the devices
> > at boot time.
>
> Yes, fair enough, but I haven't seen any problems with this. and as far as
> I know, it hasn't been an issue for our customers.
Hehe, their developers are definately annoyed by this, do they report it to
you?
>
> > The reason why you don't see this is: You at Trolltech integrate on
> > devices with a finished Linux port, this is not the reality for people
> > integrating Qtopia on new hardware.
>
> If they are still developing the device, it's easy to change if the device
> nodes change as well.
It is easy, but see above. Some one in your team says "Headset's don't work.".
You will have to find out that the numbers of /dev/input/eventX
changed...again.
z.
--
[ signature omitted ]
Message 7 in thread
Holger Freyther wrote:
> On Thursday 03 April 2008 19:42:04 Lorn Potter wrote:
>> On Thursday 03 April 2008, Holger Freyther wrote:
>
>>> The numbering of devices from userspace is random! They change at
>>> development time!
>
> Right, this means he has to stop his current task, adjust a number, compile
> (wait hours for the clean build), test, commit, go back to his original task.
You do not have to clean to rebuild any one part of Qtopia. In this case just cd
devices/ficgta01/src/plugins/qtopiacore/..
and run make ; make install
heck you don't even have to do that, in most cases, for default plugins, simply change these
environmental values, like John Faith has posted:
QWS_KEYBOARD
QWS_MOUSE_PROTO
Of course this won't work for the neo keyboard plugin, as I did not write it like that.
> This is wasting
> valuable developer time for no obvious reason. And you assume that if someone
> says "The devices goes to suspend even if power is plugged in", you
> immediately suspect the device numbers have changed... This wasting of time
> can be so easily avoided... and this wasting of time is a big annoyance.
Not as big of an annoyance as writing yet another abstraction when none is really needed, adding yet
more bloat.
>
>
>> Then the developers can change the numbers as they change. I don't see the
>> problem here.
>
> How many devices with linux have you built in your career?
>
>
>>> QLinuxInputEventDevice* inputDevice = new
>>> QLinuxInputEventDevice("Neo1973 Keyboard");
>>> connect(inputDevice, SIGNAL(keyevent(KeyEvent)), SLOT(handleKeyEvent..
>> Abstracting it at this end might make the input method plugins code look
>> better. But someone somewhere _still_ have to open /dev/input/eventX
>>
>
> Yeah, the framework (in this case Qtopia). Just take a look at the available
> ioctl's in linux/input.h. Allow the Input Device to be opened by name, or by
> the product id, vendor id tuple... This is the power of abstraction. The
> product and vendor id, and the name, and capabilities are constants, the
> number of the device is not. Do whatever you want with this information, I
> for myself consider opening /dev/input/eventFIXED_NUMBER broken.
Thats fine, but it is a perfectly valid way to access those devices, and uses less code.
>
>
>
>>> Hehe, yeah they know the name, they don't know the device number as this
>>> is fairly random and depends on the initialisation order of the devices
>>> at boot time.
>> Yes, fair enough, but I haven't seen any problems with this. and as far as
>> I know, it hasn't been an issue for our customers.
>
> Hehe, their developers are definately annoyed by this, do they report it to
> you?
>
>
>>> The reason why you don't see this is: You at Trolltech integrate on
>>> devices with a finished Linux port, this is not the reality for people
>>> integrating Qtopia on new hardware.
>> If they are still developing the device, it's easy to change if the device
>> nodes change as well.
>
> It is easy, but see above. Some one in your team says "Headset's don't work.".
> You will have to find out that the numbers of /dev/input/eventX
> changed...again.
It is not that big of deal to edit one file, and recompile the plugin. I do it all the time.
--
[ signature omitted ]
Message 8 in thread
On Thursday 03 April 2008 23:43:23 Lorn Potter wrote:
> Holger Freyther wrote:
> > Right, this means he has to stop his current task, adjust a number,
> > compile (wait hours for the clean build), test, commit, go back to his
> > original task.
>
> You do not have to clean to rebuild any one part of Qtopia. In this case
> just cd devices/ficgta01/src/plugins/qtopiacore/..
> and run make ; make install
Hehe, you want to have deterministic results, so you use the result of a
clean, deterministic autobuild... I can use sed to patch a binary too, it
doesn't make it sane or right. Anyway there is no use in arguing...
You have obviously never played the role of someone manufacturing devices, I
did in the past... With your approach you risk not submitting your change,
which might result in that a wrong gold image is sent to your factory...
>
> heck you don't even have to do that, in most cases, for default plugins,
> simply change these environmental values, like John Faith has posted:
>
> QWS_KEYBOARD
> QWS_MOUSE_PROTO
>
> Of course this won't work for the neo keyboard plugin, as I did not write
> it like that.
Right, and you still need to set the right device in the environment... and
you see that John didn't like changing of the config file/start script. Gosh,
there is an easy way to avoid this and it needs three lines of code....
>
> > This is wasting
> > valuable developer time for no obvious reason. And you assume that if
> > someone says "The devices goes to suspend even if power is plugged in",
> > you immediately suspect the device numbers have changed... This wasting
> > of time can be so easily avoided... and this wasting of time is a big
> > annoyance.
>
> Not as big of an annoyance as writing yet another abstraction when none is
> really needed, adding yet more bloat.
Right, Qtopia's bloat fight is copying and pasting the same code into N
places... (just grep event[0-9] in the sources) and the compiler will
automatically merge exact text sections into one... oh wait it doesn't...
hmm. What elese do we have to fight copy and paste bloat. ah right classes,
oh well we have a language that supports abstraction through classes....
For the neo: You have Linux Input Event handling in four places. Do you want
to tell me that copying and pasting the same code into these four places is
avoiding the bloat an abstraction would add?
>
> >> Then the developers can change the numbers as they change. I don't see
> >> the problem here.
> >
> > How many devices with linux have you built in your career?
> >
> >>> QLinuxInputEventDevice* inputDevice = new
> >>> QLinuxInputEventDevice("Neo1973 Keyboard");
> >>> connect(inputDevice, SIGNAL(keyevent(KeyEvent)),
> >>> SLOT(handleKeyEvent..
> >>
> >> Abstracting it at this end might make the input method plugins code look
> >> better. But someone somewhere _still_ have to open /dev/input/eventX
> >
> > Yeah, the framework (in this case Qtopia). Just take a look at the
> > available ioctl's in linux/input.h. Allow the Input Device to be opened
> > by name, or by the product id, vendor id tuple... This is the power of
> > abstraction. The product and vendor id, and the name, and capabilities
> > are constants, the number of the device is not. Do whatever you want with
> > this information, I for myself consider opening
> > /dev/input/eventFIXED_NUMBER broken.
>
> Thats fine, but it is a perfectly valid way to access those devices, and
> uses less code.
see above...
>
> > It is easy, but see above. Some one in your team says "Headset's don't
> > work.". You will have to find out that the numbers of /dev/input/eventX
> > changed...again.
>
> It is not that big of deal to edit one file, and recompile the plugin. I do
> it all the time.
And the code shows.
I find it frustrating to argue about something that obvious. You can add an
abstraction which will result in less code (avoiding the copy and paste) and
will always work, instead you argue that copy and paste is solving problems
and that you can always fix stuff afterwards.
And again I for myself decided to do things properly and exactly _once_, you
should consider this as well.
z.
--
[ signature omitted ]
Message 9 in thread
On Friday 04 April 2008 00:54:04 Holger Freyther wrote:
> On Thursday 03 April 2008 23:43:23 Lorn Potter wrote:
> I find it frustrating to argue about something that obvious. You can add an
> abstraction which will result in less code (avoiding the copy and paste)
> and will always work, instead you argue that copy and paste is solving
> problems and that you can always fix stuff afterwards.
>
> And again I for myself decided to do things properly and exactly _once_,
> you should consider this as well.
And here we go:
1. clean up the code to have a common coding style, indetion
2. kill dead code, from a quick grep it looks like this comes from
the greenphone? It looks like you would want to connect it to
QObject::connect(QtopiaServerApplication::instance(),
SIGNAL(shutdownRequested()), this, SLOT(shutdownRequested()))
3. Implement open by physical id
4. Add opening by name
5. Add opening by input_id
feedback welcome
z.
From 961b09e2b88064c98ec40866a32d2043a25b0693 Mon Sep 17 00:00:00 2001
From 961b09e2b88064c98ec40866a32d2043a25b0693 Mon Sep 17 00:00:00 2001
From: Holger Freyther <zecke@xxxxxxxxxxx>
Date: Fri, 4 Apr 2008 00:55:36 +0200
Subject: [PATCH] fic/gta: Clean up messy code, use the linux/input.h do not use magic values
---
devices/ficgta01/server/fichardware.cpp | 64 ++++++++++++-------------------
devices/ficgta01/server/fichardware.h | 9 +---
2 files changed, 28 insertions(+), 45 deletions(-)
diff --git a/devices/ficgta01/server/fichardware.cpp b/devices/ficgta01/server/fichardware.cpp
index 9def410..d7c1733 100644
--- a/devices/ficgta01/server/fichardware.cpp
+++ b/devices/ficgta01/server/fichardware.cpp
@@ -47,6 +47,8 @@
#include <sys/ioctl.h>
+#include <linux/input.h>
+
struct Ficgta01Input {
unsigned int dummy1;
unsigned int dummy2;
@@ -58,18 +60,16 @@ struct Ficgta01Input {
QTOPIA_TASK(Ficgta01Hardware, Ficgta01Hardware);
Ficgta01Hardware::Ficgta01Hardware()
- :vsoPortableHandsfree("/Hardware/Accessories/PortableHandsfree")
+ : m_vsoPortableHandsfree("/Hardware/Accessories/PortableHandsfree")
{
+ qWarning() << "ficgta01hardware plugin";
- qWarning()<<"ficgta01hardware plugin";
-
- vsoPortableHandsfree.setAttribute("Present", false);
-
+ m_vsoPortableHandsfree.setAttribute("Present", false);
- detectFd = ::open("/dev/input/event0", O_RDONLY|O_NDELAY, 0);
- if (detectFd >= 0) {
- auxNotify = new QSocketNotifier( detectFd, QSocketNotifier::Read, this );
- connect( auxNotify, SIGNAL(activated(int)), this, SLOT(readAuxKbdData()));
+ m_detectFd = ::open("/dev/input/event0", O_RDONLY|O_NDELAY, 0);
+ if (m_detectFd >= 0) {
+ m_auxNotify = new QSocketNotifier(m_detectFd, QSocketNotifier::Read, this);
+ connect(m_auxNotify, SIGNAL(activated(int)), this, SLOT(readAuxKbdData()));
} else {
qWarning("Cannot open /dev/input/event0 for keypad (%s)", strerror(errno));
}
@@ -81,42 +81,30 @@ Ficgta01Hardware::~Ficgta01Hardware()
void Ficgta01Hardware::readAuxKbdData()
{
-
Ficgta01Input event;
- int n = read(detectFd, &event, sizeof(Ficgta01Input));
- if(n != (int)sizeof(Ficgta01Input)) {
- return;
- }
-
- if(event.type != 5)
+ int n = read(m_detectFd, &event, sizeof(Ficgta01Input));
+ if(n != (int)sizeof(Ficgta01Input) || event.type != EV_SW)
return;
qWarning("keypressed: type=%03d, code=%03d, value=%03d (%s)",
event.type, event.code,event.value,((event.value)!=0) ? "Down":"Up");
+ // Only handle the headphone insert. value=0 (Up,Insert), value=1 (Down,Remove)
switch(event.code) {
- case 0x02: //headphone insert
- {
- // type=005, code=002, value=000 (Up) //insert
- // type=005, code=002, value=001 (Down) //out
-
- if(event.value != 0x01) {
- vsoPortableHandsfree.setAttribute("Present", true);
- vsoPortableHandsfree.sync();
- } else {
- vsoPortableHandsfree.setAttribute("Present", false);
- vsoPortableHandsfree.sync();
- }
- }
- break;
+ case SW_HEADPHONE_INSERT:
+ m_vsoPortableHandsfree.setAttribute("Present", event.value != 0x01);
+ m_vsoPortableHandsfree.sync();
+ break;
+ default:
+ break;
};
}
void Ficgta01Hardware::shutdownRequested()
{
- qLog(PowerManagement)<<" Ficgta01Hardware::shutdownRequested";
+ qLog(PowerManagement) << __PRETTY_FUNCTION__;
QtopiaIpcEnvelope e("QPE/AudioVolumeManager/Ficgta01VolumeService", "setAmpMode(bool)");
e << false;
@@ -124,33 +112,31 @@ void Ficgta01Hardware::shutdownRequested()
QFile powerFile;
QFile btPower;
- if ( QFileInfo("/sys/bus/platform/devices/gta01-pm-gsm.0/power_on").exists()) {
-//ficgta01
+ // Detect if we have a gta01 or gta02.
+ if (QFileInfo("/sys/bus/platform/devices/gta01-pm-gsm.0/power_on").exists()) {
powerFile.setFileName("/sys/bus/platform/devices/gta01-pm-gsm.0/power_on");
btPower.setFileName("/sys/bus/platform/devices/gta01-pm-bt.0/power_on");
} else {
-//ficgta02
powerFile.setFileName("/sys/bus/platform/devices/neo1973-pm-gsm.0/power_on");
btPower.setFileName("/sys/bus/platform/devices/neo1973-pm-bt.0/power_on");
}
- if( !powerFile.open(QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate)) {
- qWarning()<<"File not opened";
+ if(!powerFile.open(QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate)) {
+ qWarning() << "File not opened";
} else {
QTextStream out(&powerFile);
out << "0";
powerFile.close();
}
- if( !btPower.open(QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate)) {
- qWarning()<<"File not opened";
+ if(!btPower.open(QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate)) {
+ qWarning() << "File not opened";
} else {
QTextStream out(&btPower);
out << "0";
powerFile.close();
}
-
QtopiaServerApplication::instance()->shutdown(QtopiaServerApplication::ShutdownSystem);
}
diff --git a/devices/ficgta01/server/fichardware.h b/devices/ficgta01/server/fichardware.h
index c973dc3..c616534 100644
--- a/devices/ficgta01/server/fichardware.h
+++ b/devices/ficgta01/server/fichardware.h
@@ -45,14 +45,11 @@ public:
~Ficgta01Hardware();
private:
+ QValueSpaceObject m_vsoPortableHandsfree;
-
- QValueSpaceObject vsoPortableHandsfree;
-
- QSocketNotifier *auxNotify;
- int detectFd;
+ QSocketNotifier *m_auxNotify;
+ int m_detectFd;
-
private slots:
void readAuxKbdData();
void shutdownRequested();
--
[ signature omitted ]
From e4a790aa0a8fd39f0b2b4e4f744a22e87fa85377 Mon Sep 17 00:00:00 2001
From: Holger Freyther <zecke@xxxxxxxxxxx>
Date: Fri, 4 Apr 2008 02:59:59 +0200
Subject: [PATCH] fic/gta: Implement by opening the device by name
---
devices/ficgta01/server/fichardware.cpp | 20 +++++++++++++++-----
devices/ficgta01/server/fichardware.h | 6 +++++-
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/devices/ficgta01/server/fichardware.cpp b/devices/ficgta01/server/fichardware.cpp
index e89e998..01c012d 100644
--- a/devices/ficgta01/server/fichardware.cpp
+++ b/devices/ficgta01/server/fichardware.cpp
@@ -56,7 +56,17 @@ FicLinuxInputEventHandler::FicLinuxInputEventHandler(QObject* parent)
{
}
-bool FicLinuxInputEventHandler::open(const QByteArray& physical)
+bool FicLinuxInputEventHandler::openByPhysicalBus(const QByteArray& physical)
+{
+ return internalOpen(EVIOCGPHYS(4096), 4096, physical);
+}
+
+bool FicLinuxInputEventHandler::openByName(const QByteArray& name)
+{
+ return internalOpen(EVIOCGNAME(4096), 4096, name);
+}
+
+bool FicLinuxInputEventHandler::internalOpen(int request, int length, const QByteArray& match)
{
if (m_fd >= 0) {
::close(m_fd);
@@ -65,7 +75,7 @@ bool FicLinuxInputEventHandler::open(const QByteArray& physical)
m_fd = -1;
}
- QByteArray deviceData(4096, 0);
+ QByteArray deviceData(length, 0);
// Find a suitable device, might want to add caching
QDir dir(QLatin1String("/dev/input/"), QLatin1String("event*"));
@@ -74,12 +84,12 @@ bool FicLinuxInputEventHandler::open(const QByteArray& physical)
if (m_fd < 0)
continue;
- int ret = ioctl(m_fd, EVIOCGPHYS(deviceData.length()), deviceData.data());
+ int ret = ioctl(m_fd, request, deviceData.data());
if (ret < 0)
continue;
// match the string we got with what we wanted
- if (strcmp(deviceData.constData(), physical.constData()) == 0) {
+ if (strcmp(deviceData.constData(), match.constData()) == 0) {
break;
} else {
close(m_fd);
@@ -114,7 +124,7 @@ Ficgta01Hardware::Ficgta01Hardware()
m_vsoPortableHandsfree.setAttribute("Present", false);
m_handler = new FicLinuxInputEventHandler(this);
- if (m_handler->open("neo1973kbd/input0")) {
+ if (m_handler->openByPhysicalBus("neo1973kbd/input0")) {
connect(m_handler, SIGNAL(inputEvent(struct input_event&)),
SLOT(inputEvent(struct input_event&)));
} else {
diff --git a/devices/ficgta01/server/fichardware.h b/devices/ficgta01/server/fichardware.h
index e579529..6ac10c7 100644
--- a/devices/ficgta01/server/fichardware.h
+++ b/devices/ficgta01/server/fichardware.h
@@ -49,7 +49,8 @@ class FicLinuxInputEventHandler : public QObject {
public:
FicLinuxInputEventHandler(QObject* parent);
- bool open(const QByteArray&);
+ bool openByPhysicalBus(const QByteArray&);
+ bool openByName(const QByteArray&);
Q_SIGNALS:
void inputEvent(struct input_event&);
@@ -58,6 +59,9 @@ private Q_SLOTS:
void readData();
private:
+ bool internalOpen(int request, int length, const QByteArray&);
+
+private:
int m_fd;
QSocketNotifier* m_notifier;
};
--
[ signature omitted ]
From 39f64d78ef9166175e72fc73652e7d2d184372be Mon Sep 17 00:00:00 2001
From: Holger Freyther <zecke@xxxxxxxxxxx>
Date: Fri, 4 Apr 2008 01:55:36 +0200
Subject: [PATCH] fic/gta: Kill dead code copy and pasted from the greenphone
We do not want to handle shutting off of devices after qpe
terminated anyway. Other applications could use bluetooth
or GPS or ....
---
devices/ficgta01/server/fichardware.cpp | 39 -------------------------------
devices/ficgta01/server/fichardware.h | 1 -
2 files changed, 0 insertions(+), 40 deletions(-)
diff --git a/devices/ficgta01/server/fichardware.cpp b/devices/ficgta01/server/fichardware.cpp
index d7c1733..ae6e3b9 100644
--- a/devices/ficgta01/server/fichardware.cpp
+++ b/devices/ficgta01/server/fichardware.cpp
@@ -101,44 +101,5 @@ void Ficgta01Hardware::readAuxKbdData()
};
}
-
-void Ficgta01Hardware::shutdownRequested()
-{
- qLog(PowerManagement) << __PRETTY_FUNCTION__;
-
- QtopiaIpcEnvelope e("QPE/AudioVolumeManager/Ficgta01VolumeService", "setAmpMode(bool)");
- e << false;
-
- QFile powerFile;
- QFile btPower;
-
- // Detect if we have a gta01 or gta02.
- if (QFileInfo("/sys/bus/platform/devices/gta01-pm-gsm.0/power_on").exists()) {
- powerFile.setFileName("/sys/bus/platform/devices/gta01-pm-gsm.0/power_on");
- btPower.setFileName("/sys/bus/platform/devices/gta01-pm-bt.0/power_on");
- } else {
- powerFile.setFileName("/sys/bus/platform/devices/neo1973-pm-gsm.0/power_on");
- btPower.setFileName("/sys/bus/platform/devices/neo1973-pm-bt.0/power_on");
- }
-
- if(!powerFile.open(QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate)) {
- qWarning() << "File not opened";
- } else {
- QTextStream out(&powerFile);
- out << "0";
- powerFile.close();
- }
-
- if(!btPower.open(QIODevice::WriteOnly | QIODevice::Text | QIODevice::Truncate)) {
- qWarning() << "File not opened";
- } else {
- QTextStream out(&btPower);
- out << "0";
- powerFile.close();
- }
-
- QtopiaServerApplication::instance()->shutdown(QtopiaServerApplication::ShutdownSystem);
-}
-
#endif // QT_QWS_Ficgta01
diff --git a/devices/ficgta01/server/fichardware.h b/devices/ficgta01/server/fichardware.h
index c616534..0ac77c4 100644
--- a/devices/ficgta01/server/fichardware.h
+++ b/devices/ficgta01/server/fichardware.h
@@ -52,7 +52,6 @@ private:
private slots:
void readAuxKbdData();
- void shutdownRequested();
};
#endif // QT_QWS_FICGTA01
--
[ signature omitted ]
From bbc5d66913f1537d0bb5c244aae60be2bc74b389 Mon Sep 17 00:00:00 2001
From: Holger Freyther <zecke@xxxxxxxxxxx>
Date: Fri, 4 Apr 2008 02:46:45 +0200
Subject: [PATCH] fic/gta: Open Linux input events by physical address, use that for the keyboard
Use the physical address of the keyboard to find the right linux
input event device. The physical position of the neo1973 keyboard
does not change, the event number does. For usb opening with physical
address is not too clever as well, other things need to be done there.
---
devices/ficgta01/server/fichardware.cpp | 84 ++++++++++++++++++++++++-------
devices/ficgta01/server/fichardware.h | 37 +++++++++++--
2 files changed, 96 insertions(+), 25 deletions(-)
diff --git a/devices/ficgta01/server/fichardware.cpp b/devices/ficgta01/server/fichardware.cpp
index ae6e3b9..e89e998 100644
--- a/devices/ficgta01/server/fichardware.cpp
+++ b/devices/ficgta01/server/fichardware.cpp
@@ -47,17 +47,64 @@
#include <sys/ioctl.h>
-#include <linux/input.h>
+QTOPIA_TASK(Ficgta01Hardware, Ficgta01Hardware);
-struct Ficgta01Input {
- unsigned int dummy1;
- unsigned int dummy2;
- unsigned short type;
- unsigned short code;
- unsigned int value;
-};
+FicLinuxInputEventHandler::FicLinuxInputEventHandler(QObject* parent)
+ : QObject(parent)
+ , m_fd(-1)
+ , m_notifier(0)
+{
+}
-QTOPIA_TASK(Ficgta01Hardware, Ficgta01Hardware);
+bool FicLinuxInputEventHandler::open(const QByteArray& physical)
+{
+ if (m_fd >= 0) {
+ ::close(m_fd);
+ delete m_notifier;
+ m_notifier = 0;
+ m_fd = -1;
+ }
+
+ QByteArray deviceData(4096, 0);
+
+ // Find a suitable device, might want to add caching
+ QDir dir(QLatin1String("/dev/input/"), QLatin1String("event*"));
+ foreach(QFileInfo fileInfo, dir.entryInfoList(QDir::Files|QDir::System)) {
+ m_fd = ::open(QFile::encodeName(fileInfo.filePath()), O_RDONLY|O_NDELAY);
+ if (m_fd < 0)
+ continue;
+
+ int ret = ioctl(m_fd, EVIOCGPHYS(deviceData.length()), deviceData.data());
+ if (ret < 0)
+ continue;
+
+ // match the string we got with what we wanted
+ if (strcmp(deviceData.constData(), physical.constData()) == 0) {
+ break;
+ } else {
+ close(m_fd);
+ m_fd = -1;
+ }
+ }
+
+ if (m_fd >= 0) {
+ m_notifier = new QSocketNotifier(m_fd, QSocketNotifier::Read, this);
+ connect(m_notifier, SIGNAL(activated(int)), this, SLOT(readData()));
+ }
+
+ return m_fd >= 0;
+}
+
+void FicLinuxInputEventHandler::readData()
+{
+ struct input_event event;
+
+ int n = read(m_fd, &event, sizeof(struct input_event));
+ if(n != (int)sizeof(struct input_event))
+ return;
+
+ emit inputEvent(event);
+}
Ficgta01Hardware::Ficgta01Hardware()
: m_vsoPortableHandsfree("/Hardware/Accessories/PortableHandsfree")
@@ -66,12 +113,14 @@ Ficgta01Hardware::Ficgta01Hardware()
m_vsoPortableHandsfree.setAttribute("Present", false);
- m_detectFd = ::open("/dev/input/event0", O_RDONLY|O_NDELAY, 0);
- if (m_detectFd >= 0) {
- m_auxNotify = new QSocketNotifier(m_detectFd, QSocketNotifier::Read, this);
- connect(m_auxNotify, SIGNAL(activated(int)), this, SLOT(readAuxKbdData()));
+ m_handler = new FicLinuxInputEventHandler(this);
+ if (m_handler->open("neo1973kbd/input0")) {
+ connect(m_handler, SIGNAL(inputEvent(struct input_event&)),
+ SLOT(inputEvent(struct input_event&)));
} else {
- qWarning("Cannot open /dev/input/event0 for keypad (%s)", strerror(errno));
+ qWarning("Cannot open a device for the neo1973kbd");
+ delete m_handler;
+ m_handler = 0;
}
}
@@ -79,12 +128,9 @@ Ficgta01Hardware::~Ficgta01Hardware()
{
}
-void Ficgta01Hardware::readAuxKbdData()
+void Ficgta01Hardware::inputEvent(struct input_event& event)
{
- Ficgta01Input event;
-
- int n = read(m_detectFd, &event, sizeof(Ficgta01Input));
- if(n != (int)sizeof(Ficgta01Input) || event.type != EV_SW)
+ if(event.type != EV_SW)
return;
qWarning("keypressed: type=%03d, code=%03d, value=%03d (%s)",
diff --git a/devices/ficgta01/server/fichardware.h b/devices/ficgta01/server/fichardware.h
index 0ac77c4..e579529 100644
--- a/devices/ficgta01/server/fichardware.h
+++ b/devices/ficgta01/server/fichardware.h
@@ -29,6 +29,8 @@
#include <qvaluespace.h>
+#include <linux/input.h>
+
class QBootSourceAccessoryProvider;
class QPowerSourceProvider;
@@ -36,6 +38,31 @@ class QSocketNotifier;
class QtopiaIpcAdaptor;
class QSpeakerPhoneAccessoryProvider;
+
+/**
+ * Start of a generic implementation to deal with the linux input event
+ * handling. Open devices by physical address and later by name, product id
+ * and vendor id
+ */
+class FicLinuxInputEventHandler : public QObject {
+ Q_OBJECT
+
+public:
+ FicLinuxInputEventHandler(QObject* parent);
+ bool open(const QByteArray&);
+
+Q_SIGNALS:
+ void inputEvent(struct input_event&);
+
+private Q_SLOTS:
+ void readData();
+
+private:
+ int m_fd;
+ QSocketNotifier* m_notifier;
+};
+
+
class Ficgta01Hardware : public QObject
{
Q_OBJECT
@@ -44,14 +71,12 @@ public:
Ficgta01Hardware();
~Ficgta01Hardware();
+private Q_SLOTS:
+ void inputEvent(struct input_event&);
+
private:
QValueSpaceObject m_vsoPortableHandsfree;
-
- QSocketNotifier *m_auxNotify;
- int m_detectFd;
-
-private slots:
- void readAuxKbdData();
+ FicLinuxInputEventHandler *m_handler;
};
#endif // QT_QWS_FICGTA01
--
[ signature omitted ]
From 9e6ee3d8577ca58cf5a574a273d3a7ca409ccf35 Mon Sep 17 00:00:00 2001
From: Holger Freyther <zecke@xxxxxxxxxxx>
Date: Fri, 4 Apr 2008 03:14:53 +0200
Subject: [PATCH] fic/gta: Implement open by input_id
In an attempt to share the iterating logic I have overloaded
internalOpen to know about the difference between string and
input_id. This allows to share the code for iterating and probing
devices. This is fine as the method is private.
---
devices/ficgta01/server/fichardware.cpp | 27 ++++++++++++++++++++++++---
devices/ficgta01/server/fichardware.h | 3 ++-
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/devices/ficgta01/server/fichardware.cpp b/devices/ficgta01/server/fichardware.cpp
index 01c012d..faf12e3 100644
--- a/devices/ficgta01/server/fichardware.cpp
+++ b/devices/ficgta01/server/fichardware.cpp
@@ -49,6 +49,15 @@
QTOPIA_TASK(Ficgta01Hardware, Ficgta01Hardware);
+bool operator==(const input_id& left, const input_id& right)
+{
+ return
+ left.bustype == right.bustype &&
+ left.vendor == right.vendor &&
+ left.product == right.product &&
+ left.version == right.version;
+}
+
FicLinuxInputEventHandler::FicLinuxInputEventHandler(QObject* parent)
: QObject(parent)
, m_fd(-1)
@@ -66,7 +75,12 @@ bool FicLinuxInputEventHandler::openByName(const QByteArray& name)
return internalOpen(EVIOCGNAME(4096), 4096, name);
}
-bool FicLinuxInputEventHandler::internalOpen(int request, int length, const QByteArray& match)
+bool FicLinuxInputEventHandler::openById(const struct input_id& id)
+{
+ return internalOpen(EVIOCGID, 0, QByteArray(), &id);
+}
+
+bool FicLinuxInputEventHandler::internalOpen(unsigned request, int length, const QByteArray& match, struct input_id const *matchId)
{
if (m_fd >= 0) {
::close(m_fd);
@@ -75,7 +89,9 @@ bool FicLinuxInputEventHandler::internalOpen(int request, int length, const QByt
m_fd = -1;
}
+ const bool cgidRequest = request == EVIOCGID;
QByteArray deviceData(length, 0);
+ struct input_id deviceId;
// Find a suitable device, might want to add caching
QDir dir(QLatin1String("/dev/input/"), QLatin1String("event*"));
@@ -84,12 +100,17 @@ bool FicLinuxInputEventHandler::internalOpen(int request, int length, const QByt
if (m_fd < 0)
continue;
- int ret = ioctl(m_fd, request, deviceData.data());
+ int ret = cgidRequest ?
+ ioctl(m_fd, request, &deviceId) :
+ ioctl(m_fd, request, deviceData.data());
+
if (ret < 0)
continue;
// match the string we got with what we wanted
- if (strcmp(deviceData.constData(), match.constData()) == 0) {
+ if (cgidRequest && *matchId == deviceId) {
+ break;
+ } else if (!cgidRequest && strcmp(deviceData.constData(), match.constData()) == 0) {
break;
} else {
close(m_fd);
diff --git a/devices/ficgta01/server/fichardware.h b/devices/ficgta01/server/fichardware.h
index 6ac10c7..1956822 100644
--- a/devices/ficgta01/server/fichardware.h
+++ b/devices/ficgta01/server/fichardware.h
@@ -51,6 +51,7 @@ public:
FicLinuxInputEventHandler(QObject* parent);
bool openByPhysicalBus(const QByteArray&);
bool openByName(const QByteArray&);
+ bool openById(const struct input_id&);
Q_SIGNALS:
void inputEvent(struct input_event&);
@@ -59,7 +60,7 @@ private Q_SLOTS:
void readData();
private:
- bool internalOpen(int request, int length, const QByteArray&);
+ bool internalOpen(unsigned request, int length, const QByteArray&, struct input_id const * = 0);
private:
int m_fd;
--
[ signature omitted ]
Message 10 in thread
Hi Holger,
Holger Freyther wrote:
> On Friday 04 April 2008 00:54:04 Holger Freyther wrote:
>> On Thursday 03 April 2008 23:43:23 Lorn Potter wrote:
>
>
>> I find it frustrating to argue about something that obvious. You can add
>> an abstraction which will result in less code (avoiding the copy and
>> paste) and will always work, instead you argue that copy and paste is
>> solving problems and that you can always fix stuff afterwards.
...
Thank you for your feedback on Qtopia. We will be looking at the patch
provided as soon as possible.
For bug reports please use
http://trolltech.com/bugreport-form
in preference to sending an email to qtopia-bugs@xxxxxxxxxxxxxx The reason
being is that the web form prompts for all the information we will need to
process the bug report as quickly as possible. Once processed we will then
make the bug reports viewable via Task Tracker :
http://trolltech.com/developer/task-tracker/index_html?method=front
For your interest, to find the known Qtopia bugs use :
http://trolltech.com/developer/task-tracker/index_html?method=advsearchform
and select the Qtopia product.
Regards,
Keith
--
[ signature omitted ]
Message 11 in thread
Holger Freyther wrote:
> On Thursday 03 April 2008 23:43:23 Lorn Potter wrote:
>
>> Holger Freyther wrote:
>>
>
>
>>> Right, this means he has to stop his current task, adjust a number,
>>> compile (wait hours for the clean build), test, commit, go back to his
>>> original task.
>>>
>> You do not have to clean to rebuild any one part of Qtopia. In this case
>> just cd devices/ficgta01/src/plugins/qtopiacore/..
>> and run make ; make install
>>
>
> Hehe, you want to have deterministic results, so you use the result of a
> clean, deterministic autobuild... I can use sed to patch a binary too, it
> doesn't make it sane or right. Anyway there is no use in arguing...
>
> You have obviously never played the role of someone manufacturing devices, I
> did in the past... With your approach you risk not submitting your change,
> which might result in that a wrong gold image is sent to your factory...
>
>
>
>> heck you don't even have to do that, in most cases, for default plugins,
>> simply change these environmental values, like John Faith has posted:
>>
>> QWS_KEYBOARD
>> QWS_MOUSE_PROTO
>>
>> Of course this won't work for the neo keyboard plugin, as I did not write
>> it like that.
>>
>
> Right, and you still need to set the right device in the environment... and
> you see that John didn't like changing of the config file/start script. Gosh,
> there is an easy way to avoid this and it needs three lines of code....
>
>
>
>>> This is wasting
>>> valuable developer time for no obvious reason. And you assume that if
>>> someone says "The devices goes to suspend even if power is plugged in",
>>> you immediately suspect the device numbers have changed... This wasting
>>> of time can be so easily avoided... and this wasting of time is a big
>>> annoyance.
>>>
>> Not as big of an annoyance as writing yet another abstraction when none is
>> really needed, adding yet more bloat.
>>
Well, I really have to side Holger here. Identifying a linux event
device by just the name is the wrong approach. Of course as a developer
it's no big deal to work around this and some even take pride in it. But
it's a waste of time, nevertheless.
The scenario outlined by Holger happens frequently. The platform guys
change something in the device startup, e.g. like John said, change the
init order of some drivers and the application guys start wondering why
the keypad doesn't work any more or the charge indication is not
flashing like it should. Worse if their manager spots the problem first,
especially when you're on a tight schedule :-)
And it would be quite easy to fix the problem in a backwards compatible
way: just offer additional constructors to open an input device by
vendor:device and name. Though I consider it bad style to do operations
in a constructor which are likely to fail. I'd rather have a "bind" or
"connect" call that binds to a name or vendor:device tuple with a
boolean return value instead.
>
> Right, Qtopia's bloat fight is copying and pasting the same code into N
> places... (just grep event[0-9] in the sources) and the compiler will
> automatically merge exact text sections into one... oh wait it doesn't...
> hmm. What elese do we have to fight copy and paste bloat. ah right classes,
> oh well we have a language that supports abstraction through classes....
>
> For the neo: You have Linux Input Event handling in four places. Do you want
> to tell me that copying and pasting the same code into these four places is
> avoiding the bloat an abstraction would add?
>
Now, let's not get personal about this ...
>
>>>> Then the developers can change the numbers as they change. I don't see
>>>> the problem here.
>>>>
>>> How many devices with linux have you built in your career?
>>>
>>>
>>>>> QLinuxInputEventDevice* inputDevice = new
>>>>> QLinuxInputEventDevice("Neo1973 Keyboard");
>>>>> connect(inputDevice, SIGNAL(keyevent(KeyEvent)),
>>>>> SLOT(handleKeyEvent..
>>>>>
>>>> Abstracting it at this end might make the input method plugins code look
>>>> better. But someone somewhere _still_ have to open /dev/input/eventX
>>>>
>>> Yeah, the framework (in this case Qtopia). Just take a look at the
>>> available ioctl's in linux/input.h. Allow the Input Device to be opened
>>> by name, or by the product id, vendor id tuple... This is the power of
>>> abstraction. The product and vendor id, and the name, and capabilities
>>> are constants, the number of the device is not. Do whatever you want with
>>> this information, I for myself consider opening
>>> /dev/input/eventFIXED_NUMBER broken.
>>>
>> Thats fine, but it is a perfectly valid way to access those devices, and
>> uses less code.
>>
>
> see above...
>
It's a valid way to open a communication link at the transport level,
but not necessarily a good way to attach a plugin bound to a specific
interface class. Of course it uses less code on the Qtopia side, but a
solution using less code is not always the preferred one, especially if
the result is not adequate.
However, what Qtopia could offer if you don't want to change the plugin
interface is a method to resolve the device node from a vendor:device
tuple or a name. Given that, you'd enumerate all of /dev/input/event*,
open each one of them, retrieve name or vendor:device and return the
path to the device node that matches. A small helper class and no change
to the general abstraction model, how does that sound? Of course you
could leave it to the integrator to write that code, but he'd rather
create more and code less :-)
As with all product development, it's not the platform that makes the
profit afterwards, it's the value you add to it. So if the platform
needs to be conquered, it's a less good platform in the end...
--
[ signature omitted ]
Message 12 in thread
Matthias Welwarsky wrote:
> It's a valid way to open a communication link at the transport level,
> but not necessarily a good way to attach a plugin bound to a specific
> interface class. Of course it uses less code on the Qtopia side, but a
> solution using less code is not always the preferred one, especially if
> the result is not adequate.
What is not adequate about it? It works and it is efficient. It doesn't
matter to a user if the developer had to recompile a plugin when
developing it. As long as it works.
It is not a terribly complicated thing to recompile if/when an input
device changes.
>
> However, what Qtopia could offer if you don't want to change the plugin
> interface is a method to resolve the device node from a vendor:device
> tuple or a name. Given that, you'd enumerate all of /dev/input/event*,
> open each one of them, retrieve name or vendor:device and return the
> path to the device node that matches. A small helper class and no change
> to the general abstraction model, how does that sound? Of course you
> could leave it to the integrator to write that code, but he'd rather
> create more and code less :-)
That sounds like a very inefficient way of doing things. Not every
device Qtopia runs on has Gb to live on. This is not a desktop, we don't
have huge amounts of space, it's a space constrained device.
--
[ signature omitted ]
Message 13 in thread
Lorn Potter wrote:
> Matthias Welwarsky wrote:
>
>> It's a valid way to open a communication link at the transport level,
>> but not necessarily a good way to attach a plugin bound to a specific
>> interface class. Of course it uses less code on the Qtopia side, but
>> a solution using less code is not always the preferred one,
>> especially if the result is not adequate.
>
> What is not adequate about it? It works and it is efficient. It
> doesn't matter to a user if the developer had to recompile a plugin
> when developing it. As long as it works.
> It is not a terribly complicated thing to recompile if/when an input
> device changes.
Yes, of course. I acknowledge that it's not an unsurmountable problem.
But it is not purely technical. In any real world team you have few
skilled people that pull the whole thing and lots of "water boys". A
good platform is one that enables software development with more water
boys and less wizards. You use an operating system because you don't
want to hire an expert in TCP/IP, another one in file system design, and
so on... These people are rare and expensive, so you try to rely on as
few of them as possible. The drawback is however that now those water
boys will need to find and fix most of the bugs while not bothering too
much the few wizards while they do their, well, wizardry.
So the fragility of a platform during development becomes a quality
criterion. If the whole thing falls apart because someone changes an
"arch_initcall()" to a "late_initcall()" inside the linux kernel or
changes the load order of some modules to overcome a dependency
problem... A platform that prevents my manager from shouting at me is good.
And in the light of this, a user also doesn't care if the device he
bought runs Qtopia or not. As long as it works.
Now, I'm of course not saying that this specific problem will decide
about the fate of Qtopia as a platform. But it's better in general if
also the integrator doesn't have to rely on wizardry to do his job. The
less resources you have to involve to get a product out of the door the
better, and external resources sometimes can have steep rates :-). It
only makes your product more expensive and gives your sales people a
harder time convincing your customers :-)
>
>>
>> However, what Qtopia could offer if you don't want to change the
>> plugin interface is a method to resolve the device node from a
>> vendor:device tuple or a name. Given that, you'd enumerate all of
>> /dev/input/event*, open each one of them, retrieve name or
>> vendor:device and return the path to the device node that matches. A
>> small helper class and no change to the general abstraction model,
>> how does that sound? Of course you could leave it to the integrator
>> to write that code, but he'd rather create more and code less :-)
>
> That sounds like a very inefficient way of doing things. Not every
> device Qtopia runs on has Gb to live on. This is not a desktop, we
> don't have huge amounts of space, it's a space constrained device.
Not to be mistaken about this, but Qtopia itself is not really a
super-lean platform. Of course it beats e.g. gtk+ hands down because it
doesn't require X11, pango, cairo, etc... which makes it more efficient
in general. But still the core libraries take a couple of megabyte of
storage...
About the efficiency - it's probably not as bad as you make it sound. It
mainly one opendir(), one readdir() call for each entry in /dev/input,
one open(), one close(), one ioctl(), and one strcmp or two int16
comparisons for each "event*" found. Probably less than 1kB of binary,
and less than 200 LOC.
--
[ signature omitted ]
Message 14 in thread
On Friday 04 April 2008 12:00:07 Matthias Welwarsky wrote:
>
> Well, I really have to side Holger here. Identifying a linux event
> device by just the name is the wrong approach. Of course as a developer
> it's no big deal to work around this and some even take pride in it. But
> it's a waste of time, nevertheless.
>
> The scenario outlined by Holger happens frequently. The platform guys
> change something in the device startup, e.g. like John said, change the
> init order of some drivers and the application guys start wondering why
> the keypad doesn't work any more or the charge indication is not
> flashing like it should. Worse if their manager spots the problem first,
> especially when you're on a tight schedule :-)
>
> And it would be quite easy to fix the problem in a backwards compatible
> way: just offer additional constructors to open an input device by
> vendor:device and name. Though I consider it bad style to do operations
> in a constructor which are likely to fail. I'd rather have a "bind" or
> "connect" call that binds to a name or vendor:device tuple with a
> boolean return value instead.
Hehe, I agree. I have proposed a factory (where you could null check) or open
call, in my patch I have gone for the open calls.
>
> > Right, Qtopia's bloat fight is copying and pasting the same code into N
> > places... (just grep event[0-9] in the sources) and the compiler will
> > automatically merge exact text sections into one... oh wait it doesn't...
> > hmm. What elese do we have to fight copy and paste bloat. ah right
> > classes, oh well we have a language that supports abstraction through
> > classes....
> >
> > For the neo: You have Linux Input Event handling in four places. Do you
> > want to tell me that copying and pasting the same code into these four
> > places is avoiding the bloat an abstraction would add?
>
> Now, let's not get personal about this ...
Sorry, this was not intended to be personal. One could create a device
abstraction library, a set of possible useful classes for an integrator, with
implementations for Linux 2.4, 2.6. This could provide an alternative way to
open input devices. To not put a penalty on integrators not needing it, put
all these classes into a static library and let the linker sort out what you
don't need. This is a win win. Integrators "can code less and create more"
and you can make sure that Qtopia remains "small" and doesn't require "GB".
And regarding the code size. This is a mathematical maximation/minimizing
problem. At some point copy and pasting similar code will increase the text
size more than putting this code in a library (the maintainance burden
increases directly through copy and paste). At the KDE project we have an
estimated result for this problem. If two consumers for a feature exist
within KDE we put it in the libraries.
Do what you want, ignore this, find your own solution to this problem, I have
posted my example code to open the device by input_id, name and physical bus.
> However, what Qtopia could offer if you don't want to change the plugin
> interface is a method to resolve the device node from a vendor:device
> tuple or a name. Given that, you'd enumerate all of /dev/input/event*,
> open each one of them, retrieve name or vendor:device and return the
> path to the device node that matches. A small helper class and no change
> to the general abstraction model, how does that sound? Of course you
> could leave it to the integrator to write that code, but he'd rather
> create more and code less :-)
>
> As with all product development, it's not the platform that makes the
> profit afterwards, it's the value you add to it. So if the platform
> needs to be conquered, it's a less good platform in the end...
Thank you Matthias
z.
--
[ signature omitted ]
