NetBSD support with envsys ioctl and plist

[naguam]

New examples, simple.rs only refreshed the first battery, and this does not help checking for iterator validity for development, all.rs solves the problem. The example oneshot.rs is just for comfort avoiding loop.

NetBSD exposes battery information through one ioctl so for now we have to "waste" resource getting all sensors info for each single battery refresh. We can't assume the library user will want to refresh all bateries at the same time even if 90% of time it will probably be.

The result of the ioctl is a plist (apple format) that needs to be parsed and accessed.

The plist library only uses options, so this is the reason for utils.rs. These are all oneline functions to reduce the lenght of chains in the netbsd platform device.rs file.

NetBSD does not expose battery vendor, model, technology, cycle among other things for now, but these are optional.

Got inspiration from the FreeBSD port regarding having a better understanding on how the library works.

Thanks to unitedbsd.com community for the guidance, and shout out to https://github.com/distatus/battery/blob/master/battery_netbsd.go as well as the NetBSD devs for the great man pages and the clear code allowing to read what happens for undocumented things.

My code has comments for anyone interested to understand what happens.

By lack of hardware (only tested on one laptop), I couldn't test some specific things such as cases where battery use the watt units instead of ampere. I implemented that with the NetBSD man pages and inspiration from the FreeBSD port.

This port was only tested with a battery using the acpibat driver, but this should work with any battery that follow the same standard in the the sysmon_envsys format and fields.

[naguam]

This repository not having ffi anymore, I think the README could be updated.

starship/starship#3434

Starship might be able to have the battery feature now on NetBSD, thanks to that port.

[naguam]

I might need help with setting the build CI for NetBSD though.
Cross-rs added libexecinfo into netbsd https://github.com/cross-rs/cross/actions/runs/6527481563 but they haven't made new releases in the meantime and
https://github.com/taiki-e/install-action only uses stable release which do not contain the fix.

libexecinfo is only needed for the RUST_BACKTRACE in Cross.toml.

Three solutions:

• Just wait for the future cross-rs release (I don't know when, its been a year) ?
• Disable the backtrace for netbsd target, but still do the build.
• Use another way to setup a netbsd build env. (VM ? edge cross-rs through another way ?).

[davidkna]

@naguam If this only requires a newer build image, maybe you can point cross to a more recent one via Cross.toml?

Furthermore, you think the plist-helpers you be replaced by instead using a struct with derive(serde::Deserialize)?

[naguam]

@naguam If this only requires a newer build image, maybe you can point cross to a more recent one via Cross.toml?

Furthermore, you think the plist-helpers you be replaced by instead using a struct with derive(serde::Deserialize)?

I've never used Serde directly, It might me possible to use it, but only might as some things might be unpredictable but not necessarily wrong. A friend (Actually @ThalusA) told me the same thing, and I answered mainly the same :

I don't think I can rely on the order of the data and format, it could be if we read how they are built in kernel code, but not according to userspace documentation AFAIK.

This is why these are property lists and not plain C structs exposed throughout the ioctl like FreeBSD does in acpi.rs after all. This is XML but if the data order is not guarantied.... (NetBSD sysmon is a generic interface for many kinds of sensors, not only batteries).

Well maybe raw serde could probably handle that, but either :

• we have to make a lot of assumptions,
• or if we care about code correctness, reimplement plist with results.

So I believe just using helpers to transform Options into Result is a good compromise.
IMHO for that reason it made more sense to me to use plist as it was designed for property lists
(and plist technically uses serde under the hood, which proves my point) .

And in the end I think the solutions are equivalent and strongly believe that plist is not necessarily longer, we would still need to make comparisons per category of data.
Also as I designed a working version with plist, why changing.

The xml we parse can be obtained on NetBSD with envstat -x, and will be different depending on the computer.

Also I am not experienced in Rust and thus, using plist was the easiest way to achieve my goal.

N.b. If the trait did not impose Results or if plist proposed Results, the helpers wouldn't even have been necessary.

---

For the CI/CD I don't know, I didn't know if big changes in the github-workflow file would be relevant and wished.

I'm not sure a change in Cross.toml would help because it is throughout the github workflow that the cross-rs package is setup throughout https://github.com/taiki-e/install-action on which I don't have control of just changing the release.

[naguam]

@davidkna sorry for all the edits in my answer, I just tried to make it better.

#69 (comment)
Maybe the better solution is to disable the backtrace on NetBSD (anyway if a build fail we can check the backtrace by trying to build on a real netbsd) ? and once the new release of cross-rs arrives bring it back ?

On a localy built starship on netbsd with the change in the dependency to take my locally built starship :)

Very nice.

[davidkna]

I don't think I can rely on the order of the data and format, it could be if we read how they are built in kernel code, but not according to userspace documentation AFAIK.

serde should not care about the ordering in the plist. Nevertheless, I am not sure if the way the library maps to serde would not cause any issues.

To elaborate on how the Cross.toml file could help: Cross.toml can be used to configure the use of different docker base images for each target, as I mentioned a more recent one could solve the compile issues.

[naguam]

I think I don't understand.

Of course cross can specify other ways of building targets, but the ways of building and versions it recognises depends on the versions of cross as far as i can understand.

The way this CI is setup here,

• https://github.com/starship/rust-battery/blob/main/.github/workflows/ci.yml
  is installed by
• https://github.com/taiki-e/install-action
  which if we check.
• https://github.com/taiki-e/install-action/blob/main/manifests/cross.json
  

Gets its version from the release page tarball.
https://github.com/cross-rs/cross/releases

Which means that the available target either Dockerfile image or direct script, are not containing the fix as it is old version.

Maybe I missed something, but I probably need guidance.

Should I push in the repo the even more recent netbsd edge script, and try running it manually.

I don't think it is maintainable. Maybe my first proposal is easier (disabling the backtrace until new release comes)

[davidkna]

@naguam Cross.toml configures cross itself and is unrelated to the version that is installed (reference). It might be possible to apply the required fix-ups via this file too via pre-build.

[naguam]

Ok I think I managed to fix the CI by manually implementing the fix.

Also added clippy fixes.

Once the next proper release of the cross-rs will be published this hot fix will have to be removed (especially if NetBSD version is bumped to 10).

CI fixed.

[naguam]

By lack of hardware (only tested on one laptop), I couldn't test some specific things such as cases where battery use the watt units instead of ampere. I implemented that with the NetBSD man pages and inspiration from the FreeBSD port.

Virtualbox translates an acpi battery which is in watt unit.

I have tested my code and my assumptions were correct, so it works, no change needed.

[davidkna]

Confirmed working on real HW. LGTM, apart from two small additional suggestions.

[naguam]

For the CI thanks, I did not know I could do that

(it is probably in the documentation of cross-rs though)

However don't merge it too quickly (I wouldn't have said that yesterday)

c.f. https://www.unitedbsd.com/d/1311-battery-data-such-as-vendor-model/22

It works but it is based on descriptions so if someone changes a description it will stop working.

To solve the problem, this need to do a full rework base assumption on battery driver name, and probably use serde directly.

I don't have the motivation for now.

Also I am sorry I don't have the skills yet to review the windows pull request you made (and I am no maintainer anyway) and know nothing about windows concepts.

[davidkna]

In that case, perhaps return an error if a different driver is used for now?

[naguam]

In that case, perhaps return an error if a different driver is used for now?

Yes with the new changes only acpibat is supported and I do not rely on descriptions anymore.

Unsupported driver is ignored.

If you are happy with it, you can merge.

[davidkna]

Thanks for the contribution @naguam!