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

OpenBSD support #19

Open
svartalf opened this issue Mar 11, 2019 · 10 comments
Open

OpenBSD support #19

svartalf opened this issue Mar 11, 2019 · 10 comments
Labels
A-battery Area: battery crate C-enhancement New feature or request O-openbsd Operating system: OpenBSD

Comments

@svartalf
Copy link
Owner

This is a tracking issue for OpenBSD support.

@svartalf svartalf added C-enhancement New feature or request O-openbsd Operating system: OpenBSD A-battery Area: battery crate labels Mar 11, 2019
@svartalf
Copy link
Owner Author

@sourgrasses is working on this issue, I think.

@sourgrasses
Copy link

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

@svartalf svartalf assigned svartalf and unassigned svartalf Mar 14, 2019
@sourgrasses
Copy link

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.

Do you have a preference among those?

@svartalf
Copy link
Owner Author

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
Copy link
Owner Author

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

@svartalf svartalf pinned this issue Jun 4, 2019
@vext01
Copy link

vext01 commented May 15, 2020

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
Copy link
Owner Author

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

@vext01
Copy link

vext01 commented May 15, 2020

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

@AaronM04
Copy link

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
Copy link
Owner Author

@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.

davidkna added a commit to davidkna/rust-battery that referenced this issue Jul 31, 2023
Updates the requirements on [nix](https://github.com/nix-rust/nix) to permit the latest version.
- [Release notes](https://github.com/nix-rust/nix/releases)
- [Changelog](https://github.com/nix-rust/nix/blob/master/CHANGELOG.md)
- [Commits](nix-rust/nix@v0.25.0...v0.26.1)

---
updated-dependencies:
- dependency-name: nix
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-battery Area: battery crate C-enhancement New feature or request O-openbsd Operating system: OpenBSD
Projects
None yet
Development

No branches or pull requests

4 participants