svartalf / rust-battery

Rust crate providing cross-platform information about the notebook batteries.
https://crates.io/crates/battery
Apache License 2.0
354 stars 40 forks source link

OpenBSD support #19

Open svartalf opened 5 years ago

svartalf commented 5 years ago

This is a tracking issue for OpenBSD support.

svartalf commented 5 years ago

@sourgrasses is working on this issue, I think.

sourgrasses commented 5 years ago

Yep! Hoping to get some time this weekend to properly look at it.

sourgrasses commented 5 years ago

It looks like I can use a simple ioctl call to get basic information about the battery, but more detailed battery information will require several sysctl calls. The nix crate doesn't provide a wrapper for libc's sysctl currently, but there is another crate, sysctl, that does provide a safe wrapper to make those calls, but it doesn't seem to work correctly on OpenBSD at the moment. So I could either

Do you have a preference among those?

svartalf commented 5 years ago

Hey, @sourgrasses! I'm glad to hear that you are making a progress :)

The last two options seems to be more solid, as for me: community will benefit from that too, and we would not need to maintain these unsafe parts by ourselves anymore. Yet, it might take far more time needed to integrate functionality into sysctl or nix, so feel free to choose any option here.

As a suggestion, "unsafe stuff" can be implemented here and contributed later to the one of these crates; we can mark it as a tech debt (just create an issue linked to PR) and clean up when patches will be integrated and published.

P.S. In case you've missed, public interface had changed again, now OS implementation should conform to the traits from crate::platform::traits module.

svartalf commented 5 years ago

@sourgrasses I want to revive this thread a little bit. How are things going? Do you need any help maybe?

vext01 commented 4 years ago

Hi,

Saw a blog post about nu on lobste.rs and wanted to try it out, but sadly, as noted in this issue, doesn't build on OpenBSD due to battery abstractions.

error: Support for this target OS is not implemented yet!
 You may want to create an issue: https://github.com/svartalf/rust-battery/issues/new
  --> /home/edd/.cargo/registry/src/github.com-1ecc6299db9ec823/battery-0.7.5/src/platform/mod.rs:27:9
   |
27 | /         compile_error!("Support for this target OS is not implemented yet!\n \
28 | |             You may want to create an issue: https://github.com/svartalf/rust-battery/issues/new");
   | |___________________________________________________________________________________________________^

error[E0432]: unresolved import `crate::platform::Device`
 --> /home/edd/.cargo/registry/src/github.com-1ecc6299db9ec823/battery-0.7.5/src/types/battery.rs:5:5
  |
5 | use crate::platform::Device;
  |     ^^^^^^^^^^^^^^^^^^^^^^^ no `Device` in `platform`

error[E0432]: unresolved import `crate::platform::Iterator`
 --> /home/edd/.cargo/registry/src/github.com-1ecc6299db9ec823/battery-0.7.5/src/types/iterator.rs:3:5
  |
3 | use crate::platform::Iterator as PlatformIterator;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `Iterator` in `platform`

error[E0432]: unresolved import `crate::platform::Iterator`
 --> /home/edd/.cargo/registry/src/github.com-1ecc6299db9ec823/battery-0.7.5/src/types/manager.rs:5:5
  |
5 | use crate::platform::Iterator as PlatformIterator;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `Iterator` in `platform`

error[E0432]: unresolved import `crate::platform::Manager`
 --> /home/edd/.cargo/registry/src/github.com-1ecc6299db9ec823/battery-0.7.5/src/types/manager.rs:6:5
  |
6 | use crate::platform::Manager as PlatformManager;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `Manager` in `platform`

In the short term, can the battery support not be an optional cargo feature? I think it's more important for a shell to be ubiquitous than it is that it has all of the fancy features.

Thanks

svartalf commented 4 years ago

@vext01 you should ask Nushell developers then about making it optional

vext01 commented 4 years ago

Oops, I thought I was on the nushell repo! My bad.

AaronM04 commented 3 years ago

Hey all. I got here because I'm interested in getting starship-rs to work for OpenBSD! :)

I may be able to assist a bit. First off, it's been almost 2 years since this was written:

It looks like I can use a simple ioctl call to get basic information about the battery, but more detailed battery information will require several sysctl calls. The nix crate doesn't provide a wrapper for libc's sysctl currently, but there is another crate, sysctl, that does provide a safe wrapper to make those calls, but it doesn't seem to work correctly on OpenBSD at the moment. So I could either

  • directly use the unsafe stuff from libc here,
  • get that sysctl crate patched to work on OpenBSD and then rely on that, or
  • see if the nix crate would be interested in having that functionality added.

[..]

Is this still the case?

svartalf commented 3 years ago

@AaronM04 hey!

I'm not sure what is the current state for sysctl and nix crates (while I would prefer not to use nix crate, as it has pretty slow compilation time), so it needs some investigation first. We still can do the same suggestion as mentioned here and embed unsafe sysctl calls directly into battery crate if will it allow us to iterate faster.

From what I remember, APM interface does not provide that much information about batteries, so it hardly can be used to fill everything we need (see defined Battery struct methods), another quick investigation needed to check if sysctl interface would suit our needs.

In case of OpenBSD we will need to iterate over available sensors via sysctl(CTL_HW, HW_SENSORS, sensor_type, 0, 0) where sensor_type is one of the sensor types. From my understanding and reference implementation I'm checking, all battery-related sensors will have "battery " prefix in their sensor.description, so the first thing to do is to check and write down what exact information OpenBSD provides about batteries.

If it will be enough to fill Battery struct as is - great, and if not, we might need to re-think current battery crate public API and tune it to fit OpenBSD support into it.