Skip to content

Add CallNTPowerInformation function#1065

Merged
matthiasblaesing merged 6 commits intojava-native-access:masterfrom
dbwiddis:powrprof
Feb 17, 2019
Merged

Add CallNTPowerInformation function#1065
matthiasblaesing merged 6 commits intojava-native-access:masterfrom
dbwiddis:powrprof

Conversation

@dbwiddis
Copy link
Copy Markdown
Contributor

@dbwiddis dbwiddis commented Feb 5, 2019

No description provided.

@dbwiddis dbwiddis force-pushed the powrprof branch 2 times, most recently from 75fc595 to 4342e21 Compare February 5, 2019 05:15
@matthiasblaesing
Copy link
Copy Markdown
Member

Looks good. One question though: Was it a conscious decision to name the structures in CamelCase? Most other structures use the same name as their native counter parts.

@dbwiddis
Copy link
Copy Markdown
Contributor Author

dbwiddis commented Feb 9, 2019

Ah, good point. This was one of the first JNA structures I mapped in my project (for the battery portion) and I had chosen the Java convention at that time, before submitting code here. I'll switch it to match the native variable names.

Also, if you can hold off merging until I get a test from a user with over 64 logical processors to confirm that the function behavior matches the windows docs in terms of counting processors (this doesn't impact the base code but the test case relies on it.)

@matthiasblaesing
Copy link
Copy Markdown
Member

Sure I'll wait. Please indicate, when you think it is time to rereview this. Thank you!

Comment thread contrib/platform/test/com/sun/jna/platform/win32/PowrProfTest.java Outdated
@dbwiddis
Copy link
Copy Markdown
Contributor Author

This is good to merge. A test on an 80-logical-processor system only populated structures for the 40 processors on the current processor group, exactly as the Windows API documentation would lead one to expect.

@matthiasblaesing
Copy link
Copy Markdown
Member

Ah, good point. This was one of the first JNA structures I mapped in my project (for the battery portion) and I had chosen the Java convention at that time, before submitting code here. I'll switch it to match the native variable names.

I saw, that you changed the casing of the fields. The names of the classes themselves also differ SYSTEM_BATTERY_STATE vs. SystemBatteryState.

@dbwiddis
Copy link
Copy Markdown
Contributor Author

Fixed all the casing and went ahead and added all the necessary supporting structures for other options as well.

@dbwiddis
Copy link
Copy Markdown
Contributor Author

Hold off on merging, realized some of those structures are supposed to be in WinNT...

@dbwiddis
Copy link
Copy Markdown
Contributor Author

Should be good to go!

Comment thread contrib/platform/src/com/sun/jna/platform/win32/PowrProf.java
@matthiasblaesing matthiasblaesing merged commit 13d6c8a into java-native-access:master Feb 17, 2019
@dbwiddis dbwiddis deleted the powrprof branch February 17, 2019 21:44
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