Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-18 Thread Frederick Grose
On Tue, Oct 18, 2011 at 2:31 AM, Simon Schampijer wrote:

> On 10/17/2011 08:38 PM, Sascha Silbe wrote:
>
>> Excerpts from Simon Schampijer's message of 2011-10-17 11:07:13 +0200:
>>
>>  - powerpc is not a popular Sugar platform to make an enormous effort for
>>> supporting it
>>>
>>
>> I wouldn't consider anything we discussed an "enormous effort". With
>> that attitude PowerPC would never be a viable platform. Catch-22.
>>
>
> I knew people would jump on that. The original patch would not have had any
> impact on Sugar running on powerpc, just that the version displayed in the
> CP would not have a meaning. I could have lived with that. Still, I am
> waiting for reports of people running Sugar on powerpc...


Just for reference,
http://wiki.sugarlabs.org/go/Community/Distributions/Ubuntu/PPC
http://wiki.sugarlabs.org/go/Community/Distributions/Fedora#PowerPC_Fedora_16


> And I don't think *this* is a blocker for them.
>
>
>  - displaying all of the string on both cases (OLPC/powerpc) will likely
>>> confuse people used to the old humanized string that is displayed at
>>> current
>>>
>>> - the string does always start with "CL": CL1 for XO-1, CL1B for XO-1.5,
>>> CL2 for XO-1.75, to differentiate we can do the following:
>>>
>>> if: string.startswith('CL') string[6:13]
>>> else: display all of the string
>>>
>>
>> Sounds good enough to me. We don't even need the length check as
>> foo[6:13] will simply give an empty string if foo is too short. Might be
>> good to add a comment to that effect, though.
>>
>
> I will send a new full patch.
>
>
> Regards,
>   Simon
> __**_
> Sugar-devel mailing list
> Sugar-devel@lists.sugarlabs.**org 
> http://lists.sugarlabs.org/**listinfo/sugar-devel
>
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-18 Thread Simon Schampijer

On 10/18/2011 09:48 AM, James Cameron wrote:

Reviewed-by: James Cameron



Acked as well by Sascha on irc, pushed to master and the 0.94 branch.

http://git.sugarlabs.org/sugar/mainline/commit/64aae3b42c259af85db4051aa54c74a1b303f21b

Regards,
   Simon
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-18 Thread James Cameron
Reviewed-by: James Cameron 

-- 
James Cameron
http://quozl.linux.org.au/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


[Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread Simon Schampijer
'/ofw' is not used anymore on the XO, '/proc/device-tree' is used now
instead. Instead of just adjusting for that change we took the
chance to make the section hardware independent.

As the firmware version we display the bios version if available on non XO
hardware. On the XO we display a human readable extraction of what is
found in .../openprom/model. On other architectures (e.g. powerpc) we
display all of the content of that file.

As ethtool has become a depedency of Sugar we can now
display the wireless firmware as well on any hardware.

The serial number is often only readable by root on non-XO
hardware, that is why we do not read it. If the serial number
can not be found on an XO we display the entry as 'Not available'.

The patch has been tested on XO 1, 1.5, 1.75 and on a Thinkpad T61.

Signed-off-by: Simon Schampijer 
---
 extensions/cpsection/aboutcomputer/model.py |   24 +++---
 extensions/cpsection/aboutcomputer/view.py  |   65 +--
 2 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/extensions/cpsection/aboutcomputer/model.py 
b/extensions/cpsection/aboutcomputer/model.py
index a02eee6..431c9c0 100644
--- a/extensions/cpsection/aboutcomputer/model.py
+++ b/extensions/cpsection/aboutcomputer/model.py
@@ -35,6 +35,7 @@ _NM_DEVICE_TYPE_WIFI = 2
 
 _OFW_TREE = '/ofw'
 _PROC_TREE = '/proc/device-tree'
+_DMI_DIRECTORY = '/sys/class/dmi/id'
 _SN = 'serial-number'
 _MODEL = 'openprom/model'
 
@@ -96,18 +97,29 @@ def print_build_number():
 print get_build_number()
 
 
+def _parse_firmware_number(firmware_no):
+if firmware_no is None:
+firmware_no = _not_available
+else:
+# try to extract Open Firmware version from OLPC style version
+# string, e.g. "CL2   Q4B11  Q4B"
+if firmware_no.startswith('CL'):
+firmware_no = firmware_no[6:13]
+return firmware_no
+
+
 def get_firmware_number():
 firmware_no = None
 if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
 firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
+firmware_no = _parse_firmware_number(firmware_no)
 elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
 firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
-if firmware_no is None:
-firmware_no = _not_available
-else:
-firmware_no = re.split(' +', firmware_no)
-if len(firmware_no) == 3:
-firmware_no = firmware_no[1]
+firmware_no = _parse_firmware_number(firmware_no)
+elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'bios_version')):
+firmware_no = _read_file(os.path.join(_DMI_DIRECTORY, 'bios_version'))
+if firmware_no is None:
+firmware_no = _not_available
 return firmware_no
 
 
diff --git a/extensions/cpsection/aboutcomputer/view.py 
b/extensions/cpsection/aboutcomputer/view.py
index e5f2f32..257e165 100644
--- a/extensions/cpsection/aboutcomputer/view.py
+++ b/extensions/cpsection/aboutcomputer/view.py
@@ -16,7 +16,6 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
-import os
 from gettext import gettext as _
 
 import gtk
@@ -47,8 +46,7 @@ class AboutComputer(SectionView):
 scrollwindow.add_with_viewport(self._vbox)
 self._vbox.show()
 
-if os.path.exists('/ofw'):
-self._setup_identity()
+self._setup_identity()
 
 self._setup_software()
 self._setup_copyright()
@@ -127,37 +125,36 @@ class AboutComputer(SectionView):
 box_software.pack_start(box_sugar, expand=False)
 box_sugar.show()
 
-if os.path.exists('/ofw'):
-box_firmware = gtk.HBox(spacing=style.DEFAULT_SPACING)
-label_firmware = gtk.Label(_('Firmware:'))
-label_firmware.set_alignment(1, 0)
-label_firmware.modify_fg(gtk.STATE_NORMAL,
-  style.COLOR_SELECTION_GREY.get_gdk_color())
-box_firmware.pack_start(label_firmware, expand=False)
-self._group.add_widget(label_firmware)
-label_firmware.show()
-label_firmware_no = gtk.Label(self._model.get_firmware_number())
-label_firmware_no.set_alignment(0, 0)
-box_firmware.pack_start(label_firmware_no, expand=False)
-label_firmware_no.show()
-box_software.pack_start(box_firmware, expand=False)
-box_firmware.show()
-
-box_wireless_fw = gtk.HBox(spacing=style.DEFAULT_SPACING)
-label_wireless_fw = gtk.Label(_('Wireless Firmware:'))
-label_wireless_fw.set_alignment(1, 0)
-label_wireless_fw.modify_fg(gtk.STATE_NORMAL,
-  style.COLOR_SELECTION_GREY.get_gdk_color())
-box_wireless_fw.pack_start(label_wireless_fw, expand=False)
-self._group.add_widget(label_wireless_fw)
-label_wireless_fw.show()

Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread Simon Schampijer

On 10/17/2011 08:38 PM, Sascha Silbe wrote:

Excerpts from Simon Schampijer's message of 2011-10-17 11:07:13 +0200:


- powerpc is not a popular Sugar platform to make an enormous effort for
supporting it


I wouldn't consider anything we discussed an "enormous effort". With
that attitude PowerPC would never be a viable platform. Catch-22.


I knew people would jump on that. The original patch would not have had 
any impact on Sugar running on powerpc, just that the version displayed 
in the CP would not have a meaning. I could have lived with that. Still, 
I am waiting for reports of people running Sugar on powerpc...And I 
don't think *this* is a blocker for them.



- displaying all of the string on both cases (OLPC/powerpc) will likely
confuse people used to the old humanized string that is displayed at current

- the string does always start with "CL": CL1 for XO-1, CL1B for XO-1.5,
CL2 for XO-1.75, to differentiate we can do the following:

if: string.startswith('CL') string[6:13]
else: display all of the string


Sounds good enough to me. We don't even need the length check as
foo[6:13] will simply give an empty string if foo is too short. Might be
good to add a comment to that effect, though.


I will send a new full patch.

Regards,
   Simon
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread Sascha Silbe
Excerpts from Simon Schampijer's message of 2011-10-17 11:07:13 +0200:

> - powerpc is not a popular Sugar platform to make an enormous effort for 
> supporting it

I wouldn't consider anything we discussed an "enormous effort". With
that attitude PowerPC would never be a viable platform. Catch-22.

> - displaying all of the string on both cases (OLPC/powerpc) will likely 
> confuse people used to the old humanized string that is displayed at current
> 
> - the string does always start with "CL": CL1 for XO-1, CL1B for XO-1.5, 
> CL2 for XO-1.75, to differentiate we can do the following:
> 
> if: string.startswith('CL') string[6:13]
> else: display all of the string

Sounds good enough to me. We don't even need the length check as
foo[6:13] will simply give an empty string if foo is too short. Might be
good to add a comment to that effect, though.

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/


signature.asc
Description: PGP signature
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread James Cameron
On Mon, Oct 17, 2011 at 11:07:13AM +0200, Simon Schampijer wrote:
> - the string does always start with "CL": CL1 for XO-1, CL1B for
> XO-1.5, CL2 for XO-1.75, ...

Actually, it also starts with CL1 for XO-1.5, we didn't change it, even
though we used CL1B elsewhere.  At the time I was talking about the
identifier meaning, not our incorrect usage of it in OpenFirmware.

> if: string.startswith('CL') string[6:13]
> else: display all of the string
> 
> And we add a comment about it. Sounds a good enough compromise for
> now?

Yes.

-- 
James Cameron
http://quozl.linux.org.au/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread Peter Robinson
On Mon, Oct 17, 2011 at 10:07 AM, Simon Schampijer  wrote:
> On 10/17/2011 10:37 AM, Simon Schampijer wrote:
>>
>> On 10/17/2011 09:36 AM, James Cameron wrote:
>>>
>>> On Mon, Oct 17, 2011 at 09:23:18AM +0200, Simon Schampijer wrote:

 def _parse_firmware_number(firmware_no):
 if firmware_no is None:
 firmware_no = _not_available
 else:
 # try to extract Open Firmware version from OLPC style
 # version string, e.g. "CL2 Q4B11 Q4B"
 fields = re.split(' +', firmware_no)
 if len(fields) == 3:
 firmware_no = fields[1]
 return firmware_no
>>>
>>> Again I disagree with this method, since it expects whitespace between
>>> fields, and OpenFirmware does not guarantee whitespace.
>>>
>>> Instead, you should extract byte positions. Skip six characters, then
>>> extract seven characters. Trim any trailing spaces from the result.
>>>
>>> Mitch and I discussed this during this last week.
>>
>> Ok, I can firmware_no[6:13] but how do you differentiate then with a
>> version number you would get on powerpc like 'OpenFirmware 3'?
>>
>> Regards,
>> Simon
>
> I discussed this a bit with James on irc.
>
> - powerpc is not a popular Sugar platform to make an enormous effort for
> supporting it

Nothing to stop us improving that in the future if the situation changes?

> - displaying all of the string on both cases (OLPC/powerpc) will likely
> confuse people used to the old humanized string that is displayed at current
>
> - the string does always start with "CL": CL1 for XO-1, CL1B for XO-1.5, CL2
> for XO-1.75, to differentiate we can do the following:
>
> if: string.startswith('CL') string[6:13]
> else: display all of the string
>
> And we add a comment about it. Sounds a good enough compromise for now?

I think so.

Peter
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread Simon Schampijer

On 10/17/2011 10:37 AM, Simon Schampijer wrote:

On 10/17/2011 09:36 AM, James Cameron wrote:

On Mon, Oct 17, 2011 at 09:23:18AM +0200, Simon Schampijer wrote:

def _parse_firmware_number(firmware_no):
if firmware_no is None:
firmware_no = _not_available
else:
# try to extract Open Firmware version from OLPC style
# version string, e.g. "CL2 Q4B11 Q4B"
fields = re.split(' +', firmware_no)
if len(fields) == 3:
firmware_no = fields[1]
return firmware_no


Again I disagree with this method, since it expects whitespace between
fields, and OpenFirmware does not guarantee whitespace.

Instead, you should extract byte positions. Skip six characters, then
extract seven characters. Trim any trailing spaces from the result.

Mitch and I discussed this during this last week.


Ok, I can firmware_no[6:13] but how do you differentiate then with a
version number you would get on powerpc like 'OpenFirmware 3'?

Regards,
Simon


I discussed this a bit with James on irc.

- powerpc is not a popular Sugar platform to make an enormous effort for 
supporting it


- displaying all of the string on both cases (OLPC/powerpc) will likely 
confuse people used to the old humanized string that is displayed at current


- the string does always start with "CL": CL1 for XO-1, CL1B for XO-1.5, 
CL2 for XO-1.75, to differentiate we can do the following:


if: string.startswith('CL') string[6:13]
else: display all of the string

And we add a comment about it. Sounds a good enough compromise for now?

Regards,
   Simon

___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread Simon Schampijer

On 10/17/2011 09:36 AM, James Cameron wrote:

On Mon, Oct 17, 2011 at 09:23:18AM +0200, Simon Schampijer wrote:

def _parse_firmware_number(firmware_no):
 if firmware_no is None:
 firmware_no = _not_available
 else:
 # try to extract Open Firmware version from OLPC style
 # version string, e.g. "CL2   Q4B11 Q4B"
 fields = re.split(' +', firmware_no)
 if len(fields) == 3:
 firmware_no = fields[1]
 return firmware_no


Again I disagree with this method, since it expects whitespace between
fields, and OpenFirmware does not guarantee whitespace.

Instead, you should extract byte positions.  Skip six characters, then
extract seven characters.  Trim any trailing spaces from the result.

Mitch and I discussed this during this last week.


Ok, I can firmware_no[6:13] but how do you differentiate then with a 
version number you would get on powerpc like 'OpenFirmware 3'?


Regards,
   Simon


___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread James Cameron
On Mon, Oct 17, 2011 at 09:23:18AM +0200, Simon Schampijer wrote:
> def _parse_firmware_number(firmware_no):
> if firmware_no is None:
> firmware_no = _not_available
> else:
> # try to extract Open Firmware version from OLPC style
> # version string, e.g. "CL2   Q4B11 Q4B"
> fields = re.split(' +', firmware_no)
> if len(fields) == 3:
> firmware_no = fields[1]
> return firmware_no

Again I disagree with this method, since it expects whitespace between
fields, and OpenFirmware does not guarantee whitespace.

Instead, you should extract byte positions.  Skip six characters, then
extract seven characters.  Trim any trailing spaces from the result.

Mitch and I discussed this during this last week.

-- 
James Cameron
http://quozl.linux.org.au/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-17 Thread Simon Schampijer

On 10/09/2011 08:55 PM, Sascha Silbe wrote:

Excerpts from Simon Schampijer's message of 2011-10-09 18:01:35 +0200:


The serial number is often only readable by root on non-XO
hardware, that is why we do not read it. If the serial number
can not be found on an XO we display the entry as 'Not available'.

[...]


-def get_firmware_number():
-firmware_no = None
-if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
-firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
-elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
-firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+def _parse_olpc_firmware_number(firmware_no):
  if firmware_no is None:
  firmware_no = _not_available
  else:
@@ -111,6 +107,21 @@ def get_firmware_number():
  return firmware_no


+def get_firmware_number():
+firmware_no = None
+if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
+firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
+firmware_no = _parse_olpc_firmware_number(firmware_no)
+elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
+firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+firmware_no = _parse_olpc_firmware_number(firmware_no)
+elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'bios_version')):
+firmware_no = _read_file(os.path.join(_DMI_DIRECTORY, 'bios_version'))
+if firmware_no is None:
+firmware_no = _not_available
+return firmware_no


[...]

There still seems to be some confusion over what is available on
various systems (esp. "non-XOs"), so here are a few real-life examples:


== Apple eMac (powerpc) ==

{{{
sascha.silbe@emac:~$ uname -r
2.6.32-5-powerpc
sascha.silbe@emac:~$ tr '\0' '\n'<  /proc/device-tree/model
PowerMac4,4
sascha.silbe@emac:~$ tr '\0' '\n'<  /proc/device-tree/openprom/model
OpenFirmware 3
sascha.silbe@emac:~$ tr '\0' '\n'<  /proc/device-tree/banner-name
-bash: /proc/device-tree/banner-name: No such file or directory
sascha.silbe@emac:~$ tr '\0' '\n'<  /proc/device-tree/compatible
PowerMac4,4
MacRISC2
MacRISC
Power Macintosh
sascha.silbe@emac:~$ tr '\0' '\n'<  /proc/device-tree/serial-number |sed -e 
's/[A-Za-z]/A/g' -e 's/[0-9]/0/g' |grep -v '^$'
A0A
AAAA0
sascha.silbe@emac:~$ ls /sys/class/dmi/id
ls: cannot access /sys/class/dmi/id: No such file or directory
sascha.silbe@emac:~$ cat /proc/cpuinfo
processor   : 0
cpu : 7455, altivec supported
clock   : 999.97MHz
revision: 3.3 (pvr 8001 0303)
bogomips: 66.57
timebase: 33289815
platform: PowerMac
model   : PowerMac4,4
machine : PowerMac4,4
motherboard : PowerMac4,4 MacRISC2 MacRISC Power Macintosh
detected as : 80 (eMac)
pmac flags  : 0010
L2 cache: 256K unified
pmac-generation : NewWorld
Memory  : 640 MB
}}}


== OpenRD Ultimate (armel) ==

{{{
sascha.silbe@flatty:~$ uname -r
2.6.32-5-kirkwood
sascha.silbe@flatty:~$ ls /proc/device-tree
ls: cannot access /proc/device-tree: No such file or directory
sascha.silbe@flatty:~$ ls /sys/class/dmi/id
ls: cannot access /sys/class/dmi/id: No such file or directory
sascha.silbe@flatty:~$ cat /proc/cpuinfo
Processor   : Feroceon 88FR131 rev 1 (v5l)
BogoMIPS: 1199.30
Features: swp half thumb fastmult edsp
CPU implementer : 0x56
CPU architecture: 5TE
CPU variant : 0x2
CPU part: 0x131
CPU revision: 1

Hardware: Marvell OpenRD Ultimate Board
Revision: 
Serial  : 
}}}


== Athlon64 based PC (amd64) ==

{{{
sascha.silbe@twin:~$ uname -r
2.6.32-5-amd64
sascha.silbe@twin:~$ ls /proc/device-tree
ls: cannot access /proc/device-tree: No such file or directory
sascha.silbe@twin:~$ cat /sys/class/dmi/id/product_name
GA-MA78G-DS3H
sascha.silbe@twin:~$ cat /sys/class/dmi/id/product_version

sascha.silbe@twin:~$ cat /sys/class/dmi/id/bios_vendor
Award Software International, Inc.
sascha.silbe@twin:~$ cat /sys/class/dmi/id/bios_version
F3
}}}


== XO-1.5 (i386) ==

{{{
sascha.silbe@xo15-sascha:~$ uname -r
2.6.35.13-xo1.5-2-00905-g483bf23
sascha.silbe@xo15-sascha:~$ ls /proc/device-tree
ls: cannot access /proc/device-tree: No such file or directory
sascha.silbe@xo15-sascha:~$ tr '\0' '\n'<  /ofw/model
D4
sascha.silbe@xo15-sascha:~$ tr '\0' '\n'<  /ofw/compatible
-bash: /ofw/compatible: No such file or directory
sascha.silbe@xo15-sascha:~$ tr '\0' '\n'<  /ofw/openprom/model
CL1   Q3A64  Q3A
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/product_name
XO
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/product_version
1.5
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/bios_vendor
  IE8y2D ScD%g4r2bAIFA.
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/bios_version
OLPC Ver 1.00.15
}}}


== XO-1.75 (armel) ==

{{{
sascha.silbe@mimosa:~$ uname -r
3.0.0-mimosa-1-00159-g1abf0b6
sascha.silbe@mimosa:~$ tr '\0' '\n'<  /proc/device-tree/model
1B1
sascha.silbe@mimosa:~$ tr '\0' '\n'<  

Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-10 Thread James Cameron
On Sun, Oct 09, 2011 at 08:55:27PM +0200, Sascha Silbe wrote:
> def _parse_firmware_number(firmware_no):
> if firmware_no is None:
> return _not_available
> 
> firmware_no = firmware_no.replace('\0', ' ').strip()
> 
> # try to extract Open Firmware version from OLPC style version
> # string, e.g. "CL2   Q4B11  Q4B"
> if len(firmware_no) != 16:
> return firmware_no
> 
> fields = re.split(' +', firmware_no)
> if len(fields) != 3 or not fields[1].startswith(fields[2]):
> return firmware_no
> 
> return fields[1]

No, do not split only by space, because we use two letter suffix for
developer releases of OpenFirmware, like so:

"CL2   Q4B11jaQ4B"

Instead, separate by character position.

"CL2   " (6 characters)
"Q4B11ja" (7 characters)
"Q4B" (3 characters)

On the other hand, we might add another space, not sure.  That would
break your length check.  Waiting for a reply from Mitch.

-- 
James Cameron
http://quozl.linux.org.au/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-09 Thread Sascha Silbe
Excerpts from Simon Schampijer's message of 2011-10-09 18:01:35 +0200:

> The serial number is often only readable by root on non-XO
> hardware, that is why we do not read it. If the serial number
> can not be found on an XO we display the entry as 'Not available'.
[...]

> -def get_firmware_number():
> -firmware_no = None
> -if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
> -firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
> -elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
> -firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
> +def _parse_olpc_firmware_number(firmware_no):
>  if firmware_no is None:
>  firmware_no = _not_available
>  else:
> @@ -111,6 +107,21 @@ def get_firmware_number():
>  return firmware_no
>  
>  
> +def get_firmware_number():
> +firmware_no = None
> +if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
> +firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
> +firmware_no = _parse_olpc_firmware_number(firmware_no)
> +elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
> +firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
> +firmware_no = _parse_olpc_firmware_number(firmware_no)
> +elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'bios_version')):
> +firmware_no = _read_file(os.path.join(_DMI_DIRECTORY, 
> 'bios_version'))
> +if firmware_no is None:
> +firmware_no = _not_available
> +return firmware_no

[...]

There still seems to be some confusion over what is available on
various systems (esp. "non-XOs"), so here are a few real-life examples:


== Apple eMac (powerpc) ==

{{{
sascha.silbe@emac:~$ uname -r
2.6.32-5-powerpc
sascha.silbe@emac:~$ tr '\0' '\n' < /proc/device-tree/model
PowerMac4,4
sascha.silbe@emac:~$ tr '\0' '\n' < /proc/device-tree/openprom/model
OpenFirmware 3
sascha.silbe@emac:~$ tr '\0' '\n' < /proc/device-tree/banner-name
-bash: /proc/device-tree/banner-name: No such file or directory
sascha.silbe@emac:~$ tr '\0' '\n' < /proc/device-tree/compatible 
PowerMac4,4
MacRISC2
MacRISC
Power Macintosh
sascha.silbe@emac:~$ tr '\0' '\n' < /proc/device-tree/serial-number |sed -e 
's/[A-Za-z]/A/g' -e 's/[0-9]/0/g' |grep -v '^$'
A0A
AAAA0
sascha.silbe@emac:~$ ls /sys/class/dmi/id
ls: cannot access /sys/class/dmi/id: No such file or directory
sascha.silbe@emac:~$ cat /proc/cpuinfo 
processor   : 0
cpu : 7455, altivec supported
clock   : 999.97MHz
revision: 3.3 (pvr 8001 0303)
bogomips: 66.57
timebase: 33289815
platform: PowerMac
model   : PowerMac4,4
machine : PowerMac4,4
motherboard : PowerMac4,4 MacRISC2 MacRISC Power Macintosh
detected as : 80 (eMac)
pmac flags  : 0010
L2 cache: 256K unified
pmac-generation : NewWorld
Memory  : 640 MB
}}}


== OpenRD Ultimate (armel) ==

{{{
sascha.silbe@flatty:~$ uname -r
2.6.32-5-kirkwood
sascha.silbe@flatty:~$ ls /proc/device-tree
ls: cannot access /proc/device-tree: No such file or directory
sascha.silbe@flatty:~$ ls /sys/class/dmi/id
ls: cannot access /sys/class/dmi/id: No such file or directory
sascha.silbe@flatty:~$ cat /proc/cpuinfo 
Processor   : Feroceon 88FR131 rev 1 (v5l)
BogoMIPS: 1199.30
Features: swp half thumb fastmult edsp 
CPU implementer : 0x56
CPU architecture: 5TE
CPU variant : 0x2
CPU part: 0x131
CPU revision: 1

Hardware: Marvell OpenRD Ultimate Board
Revision: 
Serial  : 
}}}


== Athlon64 based PC (amd64) ==

{{{
sascha.silbe@twin:~$ uname -r
2.6.32-5-amd64
sascha.silbe@twin:~$ ls /proc/device-tree
ls: cannot access /proc/device-tree: No such file or directory
sascha.silbe@twin:~$ cat /sys/class/dmi/id/product_name 
GA-MA78G-DS3H
sascha.silbe@twin:~$ cat /sys/class/dmi/id/product_version 
 
sascha.silbe@twin:~$ cat /sys/class/dmi/id/bios_vendor 
Award Software International, Inc.
sascha.silbe@twin:~$ cat /sys/class/dmi/id/bios_version
F3
}}}


== XO-1.5 (i386) ==

{{{
sascha.silbe@xo15-sascha:~$ uname -r
2.6.35.13-xo1.5-2-00905-g483bf23
sascha.silbe@xo15-sascha:~$ ls /proc/device-tree
ls: cannot access /proc/device-tree: No such file or directory
sascha.silbe@xo15-sascha:~$ tr '\0' '\n' < /ofw/model
D4
sascha.silbe@xo15-sascha:~$ tr '\0' '\n' < /ofw/compatible
-bash: /ofw/compatible: No such file or directory
sascha.silbe@xo15-sascha:~$ tr '\0' '\n' < /ofw/openprom/model
CL1   Q3A64  Q3A
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/product_name 
XO
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/product_version 
1.5
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/bios_vendor
 IE8y2D ScD%g4r2bAIFA.
sascha.silbe@xo15-sascha:~$ cat /sys/class/dmi/id/bios_version 
OLPC Ver 1.00.15
}}}


== XO-1.75 (armel) ==

{{{
sascha.silbe@mimosa:~$ uname -r
3.0.0-mimosa-1-00159-g1abf0b6
sascha.silbe@mimosa:~$ tr '\0' '\n' < /proc/device-tree/model
1B1
sascha.silbe@mimo

Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-09 Thread Simon Schampijer

On 10/09/2011 05:22 PM, Sascha Silbe wrote:

Excerpts from Simon Schampijer's message of 2011-10-09 12:54:40 +0200:


@@ -58,6 +58,8 @@ def get_serial_number():
   serial_no = _read_file(os.path.join(_OFW_TREE, _SN))
   elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
   serial_no = _read_file(os.path.join(_PROC_TREE, _SN))
+else:
+return None
   if serial_no is None:
   serial_no = _not_available
   return serial_no


We are hiding the serial number completely if the kernel doesn't expose
device tree information, but show "Not available" if the file couldn't
be read or parsed. That doesn't really match the patch description.

How about:

def get_serial_number():
  if os.path.exists(os.path.join(_OFW_TREE, _SN)):
  return _read_file(os.path.join(_OFW_TREE, _SN))
  elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
  return _read_file(os.path.join(_PROC_TREE, _SN))
  elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'board_serial'):
  return _read_file(os.path.join(_DMI_DIRECTORY, 'board_serial'))
  return None


Ahh, maybe not as clear from the description. The board_serial is only
readable by root. Knowing that it does not really make sense to try to
read it.


Ah, it's really just the board_serial that's readable by root only by
default; I assumed the same was true for bios_version. In that case it
makes sense not to bother with board_serial for now (i.e. until we have
a downstream that makes board_serial readable for the user running Sugar).

I suggest the following tweak to the description:

The serial number is usually only readable by root on x86 based
hardware, so we don't bother trying. If no serial number can be
determined (for any reason) we hide that part of the section.



That is why I special cased displaying the serial number. The
behavior has not changed to previous code, so I think it is good like
that, espacially as a bug fix patch.


Well, the behaviour does change for Firmware and Wireless Firmware.
Before they were hidden, now they show 'Not available'. So why the
special-casing just for the serial number?

I don't care much either way (with a slight preference towards 'Not
available'), but we should be consistent.

Sascha


Ok, great _ I am d'accord with always displaying the entries, as well 
for the serial number. The new patch does that, good to push now?


Regards,
   Simon

___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


[Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-09 Thread Simon Schampijer
'/ofw' is not used anymore on the XO, '/proc/device-tree' is used now
instead. Instead of just adjusting for that change we took the
chance to make the section hardware independent. As the firmware
version we display the bios version if available on non XO
hardware. As ethtool has become a depedency of Sugar we can now
display the wireless firmware as well on any hardware.

The serial number is often only readable by root on non-XO
hardware, that is why we do not read it. If the serial number
can not be found on an XO we display the entry as 'Not available'.

The patch has been tested on XO 1, 1.5, 1.75 and on a Thinkpad T61.

Signed-off-by: Simon Schampijer 
---
 extensions/cpsection/aboutcomputer/model.py |   23 +++---
 extensions/cpsection/aboutcomputer/view.py  |   65 +--
 2 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/extensions/cpsection/aboutcomputer/model.py 
b/extensions/cpsection/aboutcomputer/model.py
index a02eee6..503be60 100644
--- a/extensions/cpsection/aboutcomputer/model.py
+++ b/extensions/cpsection/aboutcomputer/model.py
@@ -35,6 +35,7 @@ _NM_DEVICE_TYPE_WIFI = 2
 
 _OFW_TREE = '/ofw'
 _PROC_TREE = '/proc/device-tree'
+_DMI_DIRECTORY = '/sys/class/dmi/id'
 _SN = 'serial-number'
 _MODEL = 'openprom/model'
 
@@ -96,12 +97,7 @@ def print_build_number():
 print get_build_number()
 
 
-def get_firmware_number():
-firmware_no = None
-if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
-firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
-elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
-firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+def _parse_olpc_firmware_number(firmware_no):
 if firmware_no is None:
 firmware_no = _not_available
 else:
@@ -111,6 +107,21 @@ def get_firmware_number():
 return firmware_no
 
 
+def get_firmware_number():
+firmware_no = None
+if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
+firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
+firmware_no = _parse_olpc_firmware_number(firmware_no)
+elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
+firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+firmware_no = _parse_olpc_firmware_number(firmware_no)
+elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'bios_version')):
+firmware_no = _read_file(os.path.join(_DMI_DIRECTORY, 'bios_version'))
+if firmware_no is None:
+firmware_no = _not_available
+return firmware_no
+
+
 def print_firmware_number():
 print get_firmware_number()
 
diff --git a/extensions/cpsection/aboutcomputer/view.py 
b/extensions/cpsection/aboutcomputer/view.py
index e5f2f32..257e165 100644
--- a/extensions/cpsection/aboutcomputer/view.py
+++ b/extensions/cpsection/aboutcomputer/view.py
@@ -16,7 +16,6 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
-import os
 from gettext import gettext as _
 
 import gtk
@@ -47,8 +46,7 @@ class AboutComputer(SectionView):
 scrollwindow.add_with_viewport(self._vbox)
 self._vbox.show()
 
-if os.path.exists('/ofw'):
-self._setup_identity()
+self._setup_identity()
 
 self._setup_software()
 self._setup_copyright()
@@ -127,37 +125,36 @@ class AboutComputer(SectionView):
 box_software.pack_start(box_sugar, expand=False)
 box_sugar.show()
 
-if os.path.exists('/ofw'):
-box_firmware = gtk.HBox(spacing=style.DEFAULT_SPACING)
-label_firmware = gtk.Label(_('Firmware:'))
-label_firmware.set_alignment(1, 0)
-label_firmware.modify_fg(gtk.STATE_NORMAL,
-  style.COLOR_SELECTION_GREY.get_gdk_color())
-box_firmware.pack_start(label_firmware, expand=False)
-self._group.add_widget(label_firmware)
-label_firmware.show()
-label_firmware_no = gtk.Label(self._model.get_firmware_number())
-label_firmware_no.set_alignment(0, 0)
-box_firmware.pack_start(label_firmware_no, expand=False)
-label_firmware_no.show()
-box_software.pack_start(box_firmware, expand=False)
-box_firmware.show()
-
-box_wireless_fw = gtk.HBox(spacing=style.DEFAULT_SPACING)
-label_wireless_fw = gtk.Label(_('Wireless Firmware:'))
-label_wireless_fw.set_alignment(1, 0)
-label_wireless_fw.modify_fg(gtk.STATE_NORMAL,
-  style.COLOR_SELECTION_GREY.get_gdk_color())
-box_wireless_fw.pack_start(label_wireless_fw, expand=False)
-self._group.add_widget(label_wireless_fw)
-label_wireless_fw.show()
-wireless_fw_no = self._model.get_wireless_firmware()
-label_wireless_fw_no = gtk.Label(wireless_fw_no)
-label_wi

Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-09 Thread Sascha Silbe
Excerpts from Simon Schampijer's message of 2011-10-09 12:54:40 +0200:

> >> @@ -58,6 +58,8 @@ def get_serial_number():
> >>   serial_no = _read_file(os.path.join(_OFW_TREE, _SN))
> >>   elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
> >>   serial_no = _read_file(os.path.join(_PROC_TREE, _SN))
> >> +else:
> >> +return None
> >>   if serial_no is None:
> >>   serial_no = _not_available
> >>   return serial_no
> >
> > We are hiding the serial number completely if the kernel doesn't expose
> > device tree information, but show "Not available" if the file couldn't
> > be read or parsed. That doesn't really match the patch description.
> >
> > How about:
> >
> > def get_serial_number():
> >  if os.path.exists(os.path.join(_OFW_TREE, _SN)):
> >  return _read_file(os.path.join(_OFW_TREE, _SN))
> >  elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
> >  return _read_file(os.path.join(_PROC_TREE, _SN))
> >  elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'board_serial'):
> >  return _read_file(os.path.join(_DMI_DIRECTORY, 'board_serial'))
> >  return None
> 
> Ahh, maybe not as clear from the description. The board_serial is only 
> readable by root. Knowing that it does not really make sense to try to 
> read it.

Ah, it's really just the board_serial that's readable by root only by
default; I assumed the same was true for bios_version. In that case it
makes sense not to bother with board_serial for now (i.e. until we have
a downstream that makes board_serial readable for the user running Sugar).

I suggest the following tweak to the description:

The serial number is usually only readable by root on x86 based
hardware, so we don't bother trying. If no serial number can be
determined (for any reason) we hide that part of the section.


> That is why I special cased displaying the serial number. The 
> behavior has not changed to previous code, so I think it is good like 
> that, espacially as a bug fix patch.

Well, the behaviour does change for Firmware and Wireless Firmware.
Before they were hidden, now they show 'Not available'. So why the
special-casing just for the serial number?

I don't care much either way (with a slight preference towards 'Not
available'), but we should be consistent.

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/


signature.asc
Description: PGP signature
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


[Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-09 Thread Simon Schampijer
'/ofw' is not used anymore on the XO, '/proc/device-tree' is used now
instead. Instead of just adjusting for that change we took the
chance to make the section hardware independent. As the firmware
version we display the bios version if available on non XO
hardware. As ethtool has become a depedency of Sugar we can now
display the wireless firmware as well on all hardwares.

The serial number is often only readable by root on non-XO
hardware. If the serial number can not be found on an XO we hide
that part of the section.

The patch has been tested on XO 1, 1.5, 1.75 and on a Thinkpad T61.

Signed-off-by: Simon Schampijer 
---
 extensions/cpsection/aboutcomputer/model.py |   25 +++--
 extensions/cpsection/aboutcomputer/view.py  |   70 +-
 2 files changed, 54 insertions(+), 41 deletions(-)

diff --git a/extensions/cpsection/aboutcomputer/model.py 
b/extensions/cpsection/aboutcomputer/model.py
index a02eee6..304a697 100644
--- a/extensions/cpsection/aboutcomputer/model.py
+++ b/extensions/cpsection/aboutcomputer/model.py
@@ -35,6 +35,7 @@ _NM_DEVICE_TYPE_WIFI = 2
 
 _OFW_TREE = '/ofw'
 _PROC_TREE = '/proc/device-tree'
+_DMI_DIRECTORY = '/sys/class/dmi/id'
 _SN = 'serial-number'
 _MODEL = 'openprom/model'
 
@@ -58,6 +59,8 @@ def get_serial_number():
 serial_no = _read_file(os.path.join(_OFW_TREE, _SN))
 elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
 serial_no = _read_file(os.path.join(_PROC_TREE, _SN))
+else:
+return None
 if serial_no is None:
 serial_no = _not_available
 return serial_no
@@ -96,12 +99,7 @@ def print_build_number():
 print get_build_number()
 
 
-def get_firmware_number():
-firmware_no = None
-if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
-firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
-elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
-firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+def _parse_olpc_firmware_number(firmware_no):
 if firmware_no is None:
 firmware_no = _not_available
 else:
@@ -111,6 +109,21 @@ def get_firmware_number():
 return firmware_no
 
 
+def get_firmware_number():
+firmware_no = None
+if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
+firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
+firmware_no = _parse_olpc_firmware_number(firmware_no)
+elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
+firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+firmware_no = _parse_olpc_firmware_number(firmware_no)
+elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'bios_version')):
+firmware_no = _read_file(os.path.join(_DMI_DIRECTORY, 'bios_version'))
+if firmware_no is None:
+firmware_no = _not_available
+return firmware_no
+
+
 def print_firmware_number():
 print get_firmware_number()
 
diff --git a/extensions/cpsection/aboutcomputer/view.py 
b/extensions/cpsection/aboutcomputer/view.py
index e5f2f32..a1af4c1 100644
--- a/extensions/cpsection/aboutcomputer/view.py
+++ b/extensions/cpsection/aboutcomputer/view.py
@@ -16,7 +16,6 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
-import os
 from gettext import gettext as _
 
 import gtk
@@ -47,13 +46,15 @@ class AboutComputer(SectionView):
 scrollwindow.add_with_viewport(self._vbox)
 self._vbox.show()
 
-if os.path.exists('/ofw'):
-self._setup_identity()
+self._setup_identity()
 
 self._setup_software()
 self._setup_copyright()
 
 def _setup_identity(self):
+serial_number = self._model.get_serial_number()
+if serial_number is None:
+return
 separator_identity = gtk.HSeparator()
 self._vbox.pack_start(separator_identity, expand=False)
 separator_identity.show()
@@ -74,7 +75,7 @@ class AboutComputer(SectionView):
 box_identity.pack_start(label_serial, expand=False)
 self._group.add_widget(label_serial)
 label_serial.show()
-label_serial_no = gtk.Label(self._model.get_serial_number())
+label_serial_no = gtk.Label(serial_number)
 label_serial_no.set_alignment(0, 0)
 box_identity.pack_start(label_serial_no, expand=False)
 label_serial_no.show()
@@ -127,37 +128,36 @@ class AboutComputer(SectionView):
 box_software.pack_start(box_sugar, expand=False)
 box_sugar.show()
 
-if os.path.exists('/ofw'):
-box_firmware = gtk.HBox(spacing=style.DEFAULT_SPACING)
-label_firmware = gtk.Label(_('Firmware:'))
-label_firmware.set_alignment(1, 0)
-label_firmware.modify_fg(gtk.STATE_NORMAL,
-  style.COLOR_SELECTION_GREY.get_gdk_color())
-box_firmware.pack_start(label_firmware, expand=False)
-self.

Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-09 Thread Simon Schampijer

On 10/08/2011 09:57 PM, Sascha Silbe wrote:

Excerpts from Simon Schampijer's message of 2011-10-07 11:13:15 +0200:

'/ofw' is not used anymore on the XO, '/proc/device-tree' is used now
instead. Instead of just adjusting for that change we took the
chance to make the section hardware independent. As the firmware
version we display the bios version if available on non XO
hardware. As ethtool has become a depedency of Sugar we can now

 ^ dependency


display the wireless firmware as well on all hardwares.


There's no plural form of hardware. Maybe "any hardware"?
Not that important, though.


The serial number is often only readable by root on non-XO
hardware. If the serial number can not be found on an XO we hide
that part of the section.

The patch has been tested on XO 1, 1.5, 1.75 and on a Thinkpad T61.


I like the general approach. Thanks for considering (and even testing
on) non-XO hardware!


[extensions/cpsection/aboutcomputer/model.py]

@@ -17,7 +17,6 @@

  import os
  import logging
-import re
  import subprocess
  from gettext import gettext as _
  import errno
@@ -35,6 +34,7 @@ _NM_DEVICE_TYPE_WIFI = 2

  _OFW_TREE = '/ofw'
  _PROC_TREE = '/proc/device-tree'
+_SYS_TREE = '/sys/class/dmi/id'


Both /ofw and /proc/device/tree are mount points for the OFW device
tree. /sys/class/dmi/id OTOH contains different entries. So
_DMI_DIRECTORY might be a better name.


Is fine with me, done.


@@ -58,6 +58,8 @@ def get_serial_number():
  serial_no = _read_file(os.path.join(_OFW_TREE, _SN))
  elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
  serial_no = _read_file(os.path.join(_PROC_TREE, _SN))
+else:
+return None
  if serial_no is None:
  serial_no = _not_available
  return serial_no


We are hiding the serial number completely if the kernel doesn't expose
device tree information, but show "Not available" if the file couldn't
be read or parsed. That doesn't really match the patch description.

How about:

def get_serial_number():
 if os.path.exists(os.path.join(_OFW_TREE, _SN)):
 return _read_file(os.path.join(_OFW_TREE, _SN))
 elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
 return _read_file(os.path.join(_PROC_TREE, _SN))
 elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'board_serial'):
 return _read_file(os.path.join(_DMI_DIRECTORY, 'board_serial'))
 return None


Ahh, maybe not as clear from the description. The board_serial is only 
readable by root. Knowing that it does not really make sense to try to 
read it. That is why I special cased displaying the serial number. The 
behavior has not changed to previous code, so I think it is good like 
that, espacially as a bug fix patch.



@@ -100,14 +102,14 @@ def get_firmware_number():
  firmware_no = None
  if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
  firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
+firmware_no = firmware_no[6:13]
  elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
  firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+firmware_no = firmware_no[6:13]
+elif os.path.exists(os.path.join(_SYS_TREE, 'bios_version')):
+firmware_no = _read_file(os.path.join(_SYS_TREE, 'bios_version'))


The above assumes OLPC format (e.g. "CL2   Q4B11  Q4B") for
.../openprom/model and will break on other machines with device tree
(e.g. PPC Macs). The old code at least only did the cutting if there
actually were three space-separated parts:


Ok, will revert to old format.


  if firmware_no is None:
  firmware_no = _not_available
-else:
-firmware_no = re.split(' +', firmware_no)
-if len(firmware_no) == 3:
-firmware_no = firmware_no[1]
  return firmware_no



[extensions/cpsection/aboutcomputer/view.py]

@@ -127,37 +128,36 @@ class AboutComputer(SectionView):
  box_software.pack_start(box_sugar, expand=False)
  box_sugar.show()

-if os.path.exists('/ofw'):


I wonder if we should hide the Firmware version if it's unavailable,
like we now do for the serial number. Is it useful to know that Sugar
would display it, but can't access it? Could it confuse users if it
doesn't get mentioned at all?

Sascha



In general, I prefer the approach to show the same items all the time 
and disable them or mark them as not available like we do in the rest of 
the UI a lot (e.g. with buttons).


For the future I would as well always show the identity section, with 
the model and the serial number marked as unreadable/not available.


Regards,
   Simon
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


Re: [Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-08 Thread Sascha Silbe
Excerpts from Simon Schampijer's message of 2011-10-07 11:13:15 +0200:
> '/ofw' is not used anymore on the XO, '/proc/device-tree' is used now
> instead. Instead of just adjusting for that change we took the
> chance to make the section hardware independent. As the firmware
> version we display the bios version if available on non XO
> hardware. As ethtool has become a depedency of Sugar we can now
^ dependency

> display the wireless firmware as well on all hardwares.

There's no plural form of hardware. Maybe "any hardware"?
Not that important, though.

> The serial number is often only readable by root on non-XO
> hardware. If the serial number can not be found on an XO we hide
> that part of the section.
> 
> The patch has been tested on XO 1, 1.5, 1.75 and on a Thinkpad T61.

I like the general approach. Thanks for considering (and even testing
on) non-XO hardware!


[extensions/cpsection/aboutcomputer/model.py]
> @@ -17,7 +17,6 @@
>  
>  import os
>  import logging
> -import re
>  import subprocess
>  from gettext import gettext as _
>  import errno
> @@ -35,6 +34,7 @@ _NM_DEVICE_TYPE_WIFI = 2
>  
>  _OFW_TREE = '/ofw'
>  _PROC_TREE = '/proc/device-tree'
> +_SYS_TREE = '/sys/class/dmi/id'

Both /ofw and /proc/device/tree are mount points for the OFW device
tree. /sys/class/dmi/id OTOH contains different entries. So
_DMI_DIRECTORY might be a better name.


> @@ -58,6 +58,8 @@ def get_serial_number():
>  serial_no = _read_file(os.path.join(_OFW_TREE, _SN))
>  elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
>  serial_no = _read_file(os.path.join(_PROC_TREE, _SN))
> +else:
> +return None
>  if serial_no is None:
>  serial_no = _not_available
>  return serial_no

We are hiding the serial number completely if the kernel doesn't expose
device tree information, but show "Not available" if the file couldn't
be read or parsed. That doesn't really match the patch description.

How about:

def get_serial_number():
if os.path.exists(os.path.join(_OFW_TREE, _SN)):
return _read_file(os.path.join(_OFW_TREE, _SN))
elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
return _read_file(os.path.join(_PROC_TREE, _SN))
elif os.path.exists(os.path.join(_DMI_DIRECTORY, 'board_serial'):
return _read_file(os.path.join(_DMI_DIRECTORY, 'board_serial'))
return None



> @@ -100,14 +102,14 @@ def get_firmware_number():
>  firmware_no = None
>  if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
>  firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
> +firmware_no = firmware_no[6:13]
>  elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
>  firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
> +firmware_no = firmware_no[6:13]
> +elif os.path.exists(os.path.join(_SYS_TREE, 'bios_version')):
> +firmware_no = _read_file(os.path.join(_SYS_TREE, 'bios_version'))

The above assumes OLPC format (e.g. "CL2   Q4B11  Q4B") for
.../openprom/model and will break on other machines with device tree
(e.g. PPC Macs). The old code at least only did the cutting if there
actually were three space-separated parts:

>  if firmware_no is None:
>  firmware_no = _not_available
> -else:
> -firmware_no = re.split(' +', firmware_no)
> -if len(firmware_no) == 3:
> -firmware_no = firmware_no[1]
>  return firmware_no


[extensions/cpsection/aboutcomputer/view.py]
> @@ -127,37 +128,36 @@ class AboutComputer(SectionView):
>  box_software.pack_start(box_sugar, expand=False)
>  box_sugar.show()
>  
> -if os.path.exists('/ofw'):

I wonder if we should hide the Firmware version if it's unavailable,
like we now do for the serial number. Is it useful to know that Sugar
would display it, but can't access it? Could it confuse users if it
doesn't get mentioned at all?

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/


signature.asc
Description: PGP signature
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel


[Sugar-devel] [PATCH sugar] Control Panel: making about my computer section hardware independent, OLPC #11232

2011-10-07 Thread Simon Schampijer
'/ofw' is not used anymore on the XO, '/proc/device-tree' is used now
instead. Instead of just adjusting for that change we took the
chance to make the section hardware independent. As the firmware
version we display the bios version if available on non XO
hardware. As ethtool has become a depedency of Sugar we can now
display the wireless firmware as well on all hardwares.

The serial number is often only readable by root on non-XO
hardware. If the serial number can not be found on an XO we hide
that part of the section.

The patch has been tested on XO 1, 1.5, 1.75 and on a Thinkpad T61.

Signed-off-by: Simon Schampijer 
---
 extensions/cpsection/aboutcomputer/model.py |   12 +++--
 extensions/cpsection/aboutcomputer/view.py  |   70 +-
 2 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/extensions/cpsection/aboutcomputer/model.py 
b/extensions/cpsection/aboutcomputer/model.py
index a02eee6..5735ad0 100644
--- a/extensions/cpsection/aboutcomputer/model.py
+++ b/extensions/cpsection/aboutcomputer/model.py
@@ -17,7 +17,6 @@
 
 import os
 import logging
-import re
 import subprocess
 from gettext import gettext as _
 import errno
@@ -35,6 +34,7 @@ _NM_DEVICE_TYPE_WIFI = 2
 
 _OFW_TREE = '/ofw'
 _PROC_TREE = '/proc/device-tree'
+_SYS_TREE = '/sys/class/dmi/id'
 _SN = 'serial-number'
 _MODEL = 'openprom/model'
 
@@ -58,6 +58,8 @@ def get_serial_number():
 serial_no = _read_file(os.path.join(_OFW_TREE, _SN))
 elif os.path.exists(os.path.join(_PROC_TREE, _SN)):
 serial_no = _read_file(os.path.join(_PROC_TREE, _SN))
+else:
+return None
 if serial_no is None:
 serial_no = _not_available
 return serial_no
@@ -100,14 +102,14 @@ def get_firmware_number():
 firmware_no = None
 if os.path.exists(os.path.join(_OFW_TREE, _MODEL)):
 firmware_no = _read_file(os.path.join(_OFW_TREE, _MODEL))
+firmware_no = firmware_no[6:13]
 elif os.path.exists(os.path.join(_PROC_TREE, _MODEL)):
 firmware_no = _read_file(os.path.join(_PROC_TREE, _MODEL))
+firmware_no = firmware_no[6:13]
+elif os.path.exists(os.path.join(_SYS_TREE, 'bios_version')):
+firmware_no = _read_file(os.path.join(_SYS_TREE, 'bios_version'))
 if firmware_no is None:
 firmware_no = _not_available
-else:
-firmware_no = re.split(' +', firmware_no)
-if len(firmware_no) == 3:
-firmware_no = firmware_no[1]
 return firmware_no
 
 
diff --git a/extensions/cpsection/aboutcomputer/view.py 
b/extensions/cpsection/aboutcomputer/view.py
index e5f2f32..a1af4c1 100644
--- a/extensions/cpsection/aboutcomputer/view.py
+++ b/extensions/cpsection/aboutcomputer/view.py
@@ -16,7 +16,6 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
-import os
 from gettext import gettext as _
 
 import gtk
@@ -47,13 +46,15 @@ class AboutComputer(SectionView):
 scrollwindow.add_with_viewport(self._vbox)
 self._vbox.show()
 
-if os.path.exists('/ofw'):
-self._setup_identity()
+self._setup_identity()
 
 self._setup_software()
 self._setup_copyright()
 
 def _setup_identity(self):
+serial_number = self._model.get_serial_number()
+if serial_number is None:
+return
 separator_identity = gtk.HSeparator()
 self._vbox.pack_start(separator_identity, expand=False)
 separator_identity.show()
@@ -74,7 +75,7 @@ class AboutComputer(SectionView):
 box_identity.pack_start(label_serial, expand=False)
 self._group.add_widget(label_serial)
 label_serial.show()
-label_serial_no = gtk.Label(self._model.get_serial_number())
+label_serial_no = gtk.Label(serial_number)
 label_serial_no.set_alignment(0, 0)
 box_identity.pack_start(label_serial_no, expand=False)
 label_serial_no.show()
@@ -127,37 +128,36 @@ class AboutComputer(SectionView):
 box_software.pack_start(box_sugar, expand=False)
 box_sugar.show()
 
-if os.path.exists('/ofw'):
-box_firmware = gtk.HBox(spacing=style.DEFAULT_SPACING)
-label_firmware = gtk.Label(_('Firmware:'))
-label_firmware.set_alignment(1, 0)
-label_firmware.modify_fg(gtk.STATE_NORMAL,
-  style.COLOR_SELECTION_GREY.get_gdk_color())
-box_firmware.pack_start(label_firmware, expand=False)
-self._group.add_widget(label_firmware)
-label_firmware.show()
-label_firmware_no = gtk.Label(self._model.get_firmware_number())
-label_firmware_no.set_alignment(0, 0)
-box_firmware.pack_start(label_firmware_no, expand=False)
-label_firmware_no.show()
-box_software.pack_start(box_firmware, expand=False)
-box_firmware.show()
-
-box_wireless_fw =