Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System info #98

Merged
merged 4 commits into from
Oct 19, 2020
Merged

System info #98

merged 4 commits into from
Oct 19, 2020

Conversation

lawhead
Copy link
Collaborator

@lawhead lawhead commented Oct 13, 2020

Overview

Code spike to log more detailed system information.

Ticket

https://www.pivotaltracker.com/story/show/175193913

Contributions

  • Added more information to the system_utils#get_system_info method, including platform details, operating system name, version, architecture, processor name, processor speed, CPU count, and the amount of RAM.
  • Factored out screen resolution code to a separate method for efficiency; also fixed a deprecation warning.
  • Added code to output the system information to the session logs.

Test

  • Ran the calibration task both with full_screen set to true and false to confirm that the letter sizes were still the same after refactoring. Inspected the session logs to confirm that the new system information was written correctly.

…gging. Refactored code for getting screen resolution to remove deprecations.
@lawhead lawhead requested a review from tab-cmd October 13, 2020 00:01
Copy link
Contributor

@tab-cmd tab-cmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I added some suggestions

-------
(width, height)
"""
screen = pyglet.canvas.get_display().get_default_screen()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to change slightly to work with the updated dependencies


mem = psutil.virtual_memory()
def get_system_info() -> dict:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome - this is exactly what Barry et al. asked for!

'platform-version': platform.version(),
'architecture': platform.machine(),
'processor': platform.processor(),
'cpu_count': os.cpu_count(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we can get the CPU name in most cases. It would be worth adding! https://github.com/workhorsy/py-cpuinfo#raw-fields

@lawhead lawhead merged commit 991e398 into 1.4.3 Oct 19, 2020
@lawhead lawhead deleted the system-info branch October 19, 2020 18:28
@tab-cmd tab-cmd mentioned this pull request Jan 20, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants