Add wxSystemHardwareInfo class #24692
Conversation
Adds GetPhysicalMemory() (all platforms) and GetRemainingBatteryLifePercent() (MSW). Also, wraps wxGetFreeMemory(), wxGetCpuArchitectureName(), wxGetNativeCpuArchitectureName(), wxThread::GetCPUCount(), wxGetBatteryState(), and wxGetPowerType() free functions. Closes wxWidgets#24652
|
Note: I don't have the new source file included in the build system, I wasn't able to figure out how to do that yet. |
This is described here: https://github.com/wxWidgets/wxWidgets/blob/master/docs/contributing/how-to-add-files-to-build-system.md I think I had some issues running publicly available build of bakefile, but it was a long time ago, so... |
Co-authored-by: PB <[email protected]>
Co-authored-by: PB <[email protected]>
Nice, thanks! If the general consensus is that this interface looks OK I'll work on that next... |
vadz
left a comment
vadz
left a comment
There was a problem hiding this comment.
Sorry, I have several questions about this one. I'm not sure what the right answers are, but I'd like to at least check that we have good reasons to do things like this if we do.
BTW, have you looked at similar APIs in other libraries/languages? This might give some ideas...
There was a problem hiding this comment.
Question: do we really want to keep using short(ish) file names? I'd rather call it wx/hardwareinfo.h, it's not impossibly long and seems easier to remember.
| // ---------------------------------------------------------------------------- | ||
|
|
||
| // Information about the system's hardware | ||
| class WXDLLIMPEXP_BASE wxSystemHardwareInfo |
There was a problem hiding this comment.
I think there are 2 choices that generally make sense:
- Make this a namespace with functions inside it.
- Make this a class with static
Get()function and non-static accessors.
The latter makes sense if we ever want/need to cache some information, e.g. if GetPhysicalMemory() is called, we could also get the result of GetFreePhysicalMemory() together with it.
I'm not sure if we really need it, but having a class with just static functions doesn't seem to have any advantages compared to the first approach, so in this case I'd just use a namespace.
| static wxMemorySize GetPhysicalMemory(); | ||
| static wxMemorySize GetFreePhysicalMemory(); |
There was a problem hiding this comment.
Sorry, I don't remember if we discussed this already, but do we really need "Physical" in the names of these functions? I think just Get{Total,Free}Memory() would be good enough.
Also, I wonder if we need GetUsedMemory()?
| static wxBatteryState GetBatteryState(); | ||
| static int GetRemainingBatteryLifePercent(); | ||
| static wxPowerType GetPowerType(); |
There was a problem hiding this comment.
I'm not sure we really want to include these functions here, they don't really seem to fit ("power type" is not quite a hardware characteristic) and I think we might provide better API in some wxBattery class in wx/power.h instead.
Let's keep things minimal for now (it's always simpler to add than to remove them later) and omit this.
|
|
||
| int wxSystemHardwareInfo::GetCPUCount() | ||
| { | ||
| return wxThread::GetCPUCount(); |
There was a problem hiding this comment.
Very nitpicky but
| return wxThread::GetCPUCount(); | |
| return wxThread::GetCPUCount(); |
| @@ -0,0 +1,112 @@ | |||
| /////////////////////////////////////////////////////////////////////////////// | |||
| // Name: wx/hwinfo.h | |||
| #ifdef __WXMSW__ | ||
| #include <windows.h> | ||
| #include <sysinfoapi.h> | ||
| #elif defined(__APPLE__) | ||
| #include <sys/sysctl.h> | ||
| #elif defined(__UNIX__) | ||
| #include <sys/resource.h> | ||
| #include <sys/sysinfo.h> | ||
| #endif |
There was a problem hiding this comment.
I think it would be better to have platform-specific files rather than put everything in a single file here.
3 participants
Adds
GetPhysicalMemory()(all platforms),IsRunningNatively()(all platforms), andGetRemainingBatteryLifePercent()(MSW).Also, wraps
wxGetFreeMemory(),wxGetCpuArchitectureName(),wxGetNativeCpuArchitectureName(),wxThread::GetCPUCount(),wxGetBatteryState(), andwxGetPowerType()free functions.Closes #24652
I designed it as a simple collection of static functions because, unlike
wxPlatformInfo, most of the things in here are constantly changing.