Comment 1 by nvdakor on 2013-12-17 03:57
Hi,
This could also solve an issue where Windows 8.1 was reported as 8. This is a known issue in Python as well.

Comment 3 by leonarddr on 2015-02-03 17:22
i created branch t3724 on http://bitbucket.org/leonardder/nvda.git where logging and update check queries report major.minor.build service_pack_major.service_pack_minor product_type as the windows version. For windows 8.1, the string is now '''6.2.9200 0.0 1'''. This is actually not the nicest representation i could think of.

Comment 4 by jteh on 2015-02-03 23:06
Thanks! It'd be good if this was human readable. So, if service_pack_major is 0, don't include it. If it's non-0, prefix it with "service pack". Also, we could translate product_type to the word "server" or "workstation".

I'm not sure where you put this, but it could go in the winVersion module. It should probably only be calculated once and then cached.

Comment 5 by leonarddr (in reply to comment 4) on 2015-02-05 14:10
I updated t3724:

• added two variables to winVersion.py
  • winVersion.winVersion contains the result of sys.getwindowsversion()
  • winVersion.longWinVersion contains service pack and product type as requested in this comment.
• Also, where sys.getwindowsversion was called, now winVersion.winVersion is used. This doesn't yet apply to easeOfAccess, which should be done as soon as this branch is merged into next. If it will ever be...
  Please especially review longWinVersion, I have the impression that this could have been coded better.

Comment 6 by leonarddr on 2015-02-05 16:09
After further investigation, i have the suggestion to Use platform module instead of sys to retrieve the longWinVersion. For example, platform.win32_ver returns on XP: ('XP', '5.1.2600', 'SP2', 'Multiprocessor Free'). On 8.1: ('post2012Server', '6.3.9600', '', 'Multiprocessor Free')
I think the string should read "XP (5.1.2600) service pack 2 workstation". Only 8.1 would need a workaround, as post2012server is quite uggly. The others return expected info (vista, 7, 8, don't know about server versions). We could also just stick to the current situation of course, but I think XP is a bit more readable than just 5.1.

Comment 7 by jteh on 2015-02-05 22:57
I'd prefer to stick with the version numbering (e.g. 5.1), mostly because our existing updates use that scheme and changing it would break statistics.

Comment 8 by leonarddr on 2015-02-06 07:57
In that case, I'd say this code should do. Could you please add NeedsCodeReview keyword?

Comment 9 by jteh on 2015-02-06 11:33
Thanks!

Comment 10 by jteh on 2015-02-20 05:56
Very nice work. It'd be great if you could make the following minor changes:

1. Change the name longWinVersion to winVersionText, as I think the meaning is clearer.
2. Exclude service pack minor from winVersionText if it is 0; e.g. so we get "service pack 1" instead of "service pack 1.0", but we still get "service pack 1.1" if appropriate.

Thanks.

Comment 11 by leonarddr (in reply to comment 10) on 2015-02-20 08:58
Replying to jteh:

Very nice work. It'd be great if you could make the following minor changes:

1. Change the name longWinVersion to winVersionText, as I think the meaning is clearer.
2. Exclude service pack minor from winVersionText if it is 0; e.g. so we get "service pack 1" instead of "service pack 1.0", but we still get "service pack 1.1" if appropriate.

Thanks.

Done as requested. The statement for retrieving the right service pack string is now compacted using inline if statements.

Comment 15 by James Teh <jamie@... on 2015-04-09 07:22
In [4b9a926]:

Merge branch 't3724' into next

Incubates #3724.

Changes:
Added labels: incubating

Comment 16 by jteh on 2015-04-09 07:26
Thanks!

While I'm normally a big fan of code brevity, I found that service pack line to be difficult to read, so I ended up rewriting it without the inline ifs. I also changed the list of product types to a tuple, as it doesn't need to be mutable (though this has no practical effect).

Comment 17 by James Teh <jamie@... on 2015-04-23 04:14
In [fcd157e]:

Improve Windows version reporting for logging and update checks.
1. The service pack string is now always English for consistency.
2. The version logged by NVDA is now a nice, easy to read string.
3. The version also includes whether this is a workstation or server version of Windows.
   Fixes #3724.

Changes:
Removed labels: incubating
State: closed

Comment 18 by jteh on 2015-04-23 04:16
Changes:
Milestone changed from next to 2015.2