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

Batteries iterator order #65

Open
lucassardois opened this issue Jul 1, 2020 · 4 comments
Open

Batteries iterator order #65

lucassardois opened this issue Jul 1, 2020 · 4 comments
Labels
A-battery Area: battery crate C-enhancement New feature or request

Comments

@lucassardois
Copy link

lucassardois commented Jul 1, 2020

Is the batteries iterator having a fixed order? From my tests it seem to ne be the case. acpi gives me:

Battery 0: Discharging, 15%, 00:28:40 remaining
Battery 1: Full, 100%

But the batteries iterator first give me Battery 1 then Battery 0. I didn't find anything about this in the documentation.

@svartalf svartalf added A-battery Area: battery crate C-enhancement New feature or request labels Jul 2, 2020
@svartalf
Copy link
Owner

svartalf commented Jul 2, 2020

Hey, @loustak, great question!

There are no guarantees provided right now, I clarified that in 9b7b412, thanks for pointing that out.

In general, I don't think that Batteries iterator should yield them in some particular order, because some underline OS implementations are not providing it too, so it will require some kind of internal buffering at least. Additionaly, it is not required by current battery crate consumers right now, so introducing it will hit their usage performance with no gain in return.

We can potentially expose an OS-specific identifier, which could be used for sorting, if it might be helpful somehow:

OS Type and source
Linux Can be parsed from the /sys/class/power_supply/BAT0 path
macOS IOKit objects have some id field of i32 type, I think?
Windows BatteryTag field of u32 type from BATTERY_QUERY_INFORMATION struct
FreeBSD unit field of libc::c_int type from ACPI data

And of course, it should be future-proof, as some OSes (which can be implemented later) might not have any identifier of that kind at all.

Do you have a specific case where you need the guaranteed order?

@lucassardois
Copy link
Author

Thanks for the answer!

My need is not much to have the iterator already sorted but more of a way to sort it.

You crate is beeing used in the following project to list batteries available on the system and print informations in the status bar. The idea is to print the batteries informations, and I wanted to sort them so the order stay consistent whatever happen.

@svartalf
Copy link
Owner

svartalf commented Jul 2, 2020

I see, thanks for explaining it. So far I see two ways to solve this issue:

  1. Introduce OS-specific traits similar to what std does (unix' FileExt trait, for example) with some kind of fn id(&self) -> {tag} method, where {tag} is the type from the table above.

For Linux it will look similar to this then:

#[cfg(target_os = "linux")]
pub trait BatteryExt {
    fn id(&self) -> u64;  // Not sure about the exact device id type right now
}

impl BatteryExt for crate::Battery {
    fn id(&self) -> u64 {
        self.inner_os_specific_implementation.id
    }
}

so you will need to use them with #[cfg] attributes, let's say, with cfg-if crate help:

let battery_id = cfg_if::cfg_if! {
    if cfg!(target_os = "linux") {
        use battery::os::linux::BatteryExt;
        battery.id()
    } elif cfg!(target_os = "windows") {
        use battery::os::windows::BatteryExt;
        battery.id()
    } elif /* and so on… */
  1. Another option I see is to introduce some opaque Tag newtype, which will contain OS-specific battery unique id and implements Hash + PartialOrd + Ord (and maybe something else):
impl Battery {
    pub tag(&self) -> Tag {
        // random fake code
        Tag(self.linux_implementation.sysfs_path.name.parse_int_from_it())
    }
}

It seems better from the UX point, but I'm not sure if it is fully portable yet, so it needs to be investigated first.

@lucassardois
Copy link
Author

I like idea 2 because, as you said, it make it opaque to the user of the crate.

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
Projects
None yet
Development

No branches or pull requests

2 participants