Page MenuHomeFreeBSD

Manual for ACPI battery subsystm
ClosedPublic

Authored by takawata on Tue, Nov 26, 8:42 AM.

Details

Reviewers
None
Group Reviewers
manpages
Commits
rS355574: Add ACPI battery subsystem man page.
Summary

ACPI battery subsystem was lack of document.
So I write man page for it.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

takawata created this revision.Tue, Nov 26, 8:42 AM
bcr added a subscriber: bcr.Tue, Nov 26, 9:29 AM

Thank you for writing the man page!
I've found a typo. Can you run textproc/igor and "mandoc -Tlint" over the man page and see if they turn up any warnings?

share/man/man4/acpi_battery.4
67 ↗(On Diff #64880)

s/Retruns/Returns/

takawata updated this revision to Diff 64884.Tue, Nov 26, 10:14 AM

Fix errors: typo and mandoc -Tlint.

takawata updated this revision to Diff 64885.Tue, Nov 26, 10:17 AM

Forget to change description from acpi_thermal.4, which was based.

danfe added a subscriber: danfe.Wed, Nov 27, 1:48 PM
danfe added inline comments.
share/man/man4/acpi_battery.4
42 ↗(On Diff #64885)

You should get confirmation from the native speaker, but it looks like the is missing before one and AML.

97 ↗(On Diff #64885)

Ditto before the ACPI.

109 ↗(On Diff #64885)

Apparently missing word between units (used?) by the battery...

120 ↗(On Diff #64885)

Is a really needed here before capacity and rate (below)?

139 ↗(On Diff #64885)

by, not in?

144 ↗(On Diff #64885)

Maybe just notification is sent to the system (without leading a)?

148 ↗(On Diff #64885)

Shouldn't it be spelled as .Va low and .Va warning so they stand out? Ditto below for warning and full.

158 ↗(On Diff #64885)

Hmm, in string does not look grammatically correct to me (here and below).

167 ↗(On Diff #64885)

... given by the ACPI _BST ..., perhaps?

223 ↗(On Diff #64885)

life of battery sounds a bit weird, as if the battery would really die after it discharges. :-) Even battery life is better, if it's hard to avoid using file altogether.

240 ↗(On Diff #64885)

Shouldn't the the be moved to the right? That is, Note that notifications are supported only by the CMB

takawata updated this revision to Diff 65002.Thu, Nov 28, 10:43 AM

Thank you for your comment. I've updated it following your advice.

bcr added a comment.Mon, Dec 2, 12:00 PM

I found two instances of superfluous spaces. I think once they are fixed, we can go ahead and commit the man page.

share/man/man4/acpi_battery.4
120 ↗(On Diff #65002)

There is an extra space here between "that" and "capacity".

121 ↗(On Diff #65002)

and here is another extra space.

takawata updated this revision to Diff 65157.Tue, Dec 3, 10:28 AM

Thanks. corrected.

Looks good, thanks for writing this! Some suggestions inline. Please also bump .Dd before you commit this.

share/man/man4/acpi_battery.4
42 ↗(On Diff #64885)

"The former is accessed by AML"

47 ↗(On Diff #64885)

"an" or "the" SMBus interface perhaps?

48 ↗(On Diff #64885)

"both in" can be removed

52 ↗(On Diff #64885)

as -> as the

56 ↗(On Diff #64885)

for -> for the

67 ↗(On Diff #64885)

number of the -> number of

124 ↗(On Diff #65157)

The battery's

136 ↗(On Diff #65157)

Secondary

143 ↗(On Diff #65157)

Remove "to"

151 ↗(On Diff #65157)

Missing space between warning and .

160 ↗(On Diff #65157)

These two lines should be joined, e.g. ".Va full ."

192 ↗(On Diff #65157)

depends

239 ↗(On Diff #65157)

Either "via the devd(8) interface" or " via devd(8)" (with the .Xr markup that's already there)

takawata updated this revision to Diff 65291.Fri, Dec 6, 4:48 AM

Update to reflect comments.

This revision was not accepted when it landed; it landed in state Needs Review.Tue, Dec 10, 2:19 AM
This revision was automatically updated to reflect the committed changes.