-
Notifications
You must be signed in to change notification settings - Fork 5
Capture Interface type, motherboard model #110
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
base: dev
Are you sure you want to change the base?
Conversation
|
@jamesmarkchan pls take a look at the PR. |
src/jdiskmark/Util.java
Outdated
|
|
||
| line=line.trim(); | ||
|
|
||
| if(line.isEmpty()) { continue;} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor but if you could add spaces around the keywords like if to avoid others formatting your code and you don't loose the line authorship. same with the = assignment operator and before the {. :)
src/jdiskmark/Util.java
Outdated
|
|
||
| try { | ||
| if (App.os.startsWith("Windows")) { | ||
| process = new ProcessBuilder("wmic", "diskdrive", "get", "InterfaceType").start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/jdiskmark/Util.java
Outdated
|
|
||
| if(line.isEmpty()) { continue;} | ||
|
|
||
| if(line.equalsIgnoreCase("InterfaceType") || line.equalsIgnoreCase("TRAN")|| line.equalsIgnoreCase("NAME")) { continue;} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I've found helpful for code reviews and future maintenance is to show the normal output of the command being used in the java docs or comments. I'm definitely interested in collecting more information on the drives interface so happy to see you are looking at this. worth looking at disk-model.ps1 to see how the drive and target attribute column can be queried in order to create the mapping. might be able to do it with command line also. @jslcom might have thoughts on the linux solutions.
|
|
||
| try { | ||
| if (App.os.startsWith("Windows")) { | ||
| process = new ProcessBuilder("wmic", "baseboard", "get", "Product").start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } else if (App.os.contains("Linux")) { | ||
| process = new ProcessBuilder("cat", "/sys/devices/virtual/dmi/id/board_name").start(); | ||
| } else if (App.os.startsWith("Mac OS")) { | ||
| process = new ProcessBuilder("system_profiler", "SPHardwareDataType").start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/jdiskmark/Util.java
Outdated
| } else if (App.os.contains("Linux")) { | ||
| process = new ProcessBuilder("lsblk", "-d", "-o", "tran").start(); | ||
| } else if (App.os.startsWith("Mac OS")) { | ||
| process = new ProcessBuilder("diskutil", "info", "disk0").start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@Avinash1423 i'm not sure if the title and graph title are getting too cluttered but nice to explore making this info easily read. For certain we want this information stored as part of the benchmark and accessible to us and the user.
my thoughts to make look more consistent are:
|
Signed-off-by: Avinash Niyas <avinashNiyaz1423@gmail.com>
Signed-off-by: Avinash Niyas <avinashNiyaz1423@gmail.com>
88d6f95 to
d9dc368
Compare
|
@jamesmarkchan i made some changes, please take a look. |
|
@Avinash1423 definitely interesting to see how this looks, ty for preparing it, it is possible that it is a little cluttered at the top so we'll have to think about it. we still have some time before the target 1.0.0 release.
The above screenshot is on my old and aging intel NUC running Fedora. i'm working on a refactor of the data model where it will become pretty clear where this information can be captured so it is sent back to our community portal. i'll ping you once i get that working in case you would like to pull updates and incorporate that into this branch. after the information is collected at our community portal the web page is much more spacious for laying out more information and @JasmineRRod can think about where this information can be displayed. |
|
@Avinash1423 if you pull from 84dcfae#diff-34af0e420f17e3cfbdcaa620020617221c57b66aef920f5f34028cfe2763d285 |






#87
Added a method getMotherBoardName and getInterfaceType methods in Util.java .
These methods are sensitive to platform.
Mother Board Name is displayed along w cpu info in title area
Interface Type is diplayed along with disk info