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

Add CallNTPowerInformation function #1065

Merged
merged 6 commits into from
Feb 17, 2019

Conversation

dbwiddis
Copy link
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
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
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
Member

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

@dbwiddis
Copy link
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
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
Contributor Author

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

@dbwiddis
Copy link
Contributor Author

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

@dbwiddis
Copy link
Contributor Author

Should be good to go!

@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