Page MenuHomeFreeBSD

beep(1): Initial version of utility to create terminal beep via soundcard.
ClosedPublic

Authored by hselasky on Oct 26 2021, 5:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 21, 3:54 PM
Unknown Object (File)
Sun, Dec 15, 6:20 PM
Unknown Object (File)
Thu, Dec 12, 7:16 PM
Unknown Object (File)
Wed, Dec 11, 9:26 PM
Unknown Object (File)
Sat, Nov 30, 11:08 AM
Unknown Object (File)
Wed, Nov 27, 9:05 AM
Unknown Object (File)
Nov 17 2024, 11:09 AM
Unknown Object (File)
Nov 16 2024, 1:29 PM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

hselasky created this revision.
hselasky added a reviewer: emaste.

Waveform can be adjusted with the WAVE_POWER macro. 0.5 means pure sinewave.

beep_001.png (370×621 px, 67 KB)

beep_002.png (370×621 px, 36 KB)

usr.sbin/beep/beep.c
63

We have bool now

156

B missing

hselasky marked 2 inline comments as done.

Add support and documentation for -B option.

Make gain logarithmic - suggested by @emaste .

Add missing free() - makes valgrind happy.

Advanced usage:

beep -B && beep -B -F 660 && beep -B -F 330

Excuse me, but I only have stylistic questions.

usr.sbin/beep/beep.c
28–40

Sorry, I'm a bit too tired to double check against style(9), but these seem to be in wrong order.

68

Why the ugly underscores?

91

Is this Gray encoding, after Frank Gray?

165

I may be wrong, but I think we want to avoid using atoi() in new code.

usr.sbin/beep/beep.8
4

Aren't we dropping this from wherever we can legally?

usr.sbin/beep/beep.c
194

is this daemon(3)? That returns an int, which would mandate an explicit comparison.

usr.sbin/beep/beep.c
165

Indeed atoi(3) says

The atoi() function has been deprecated by strtol() and should not be
used in new code.
usr.sbin/beep/beep.8
4

Yeah the standard copyright template for new code no longer has this. It's not a big deal but serves no purpose so if there's no other reason to include it (e.g. your employer requires it) then I'd leave it out.

Thanks for all comments. I'll address them later today.

usr.sbin/beep/beep.c
91

Yes, I'll fix the spelling.

usr.sbin/beep/beep.c
68

I'll fix it.

hselasky added inline comments.
usr.sbin/beep/beep.8
4

Fixed.

usr.sbin/beep/beep.c
28–40

All includes are now sorted by name.

165

Fixed.

hselasky marked 2 inline comments as done.

Handle comments.

Very minor nits, ok to ignore.

usr.sbin/beep/beep.c
39

"Kernel includes come first"

98

Subjective note, but this looks awkward to me, especially with the quotes. I would just call it as everyone else: "apply Gray code".

usr.sbin/beep/beep.c
39

clang-format says:

include <sys/soundcard.h>

#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <math.h>
#include <paths.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

Will handle the comments tomorrow.

After some IRC discussion, should be /usr/bin/beep, beep(1) perhaps?

hselasky marked an inline comment as done.
hselasky retitled this revision from beep(8): Initial version of utility to create terminal beep via soundcard. to beep(1): Initial version of utility to create terminal beep via soundcard..

Address all comments so far.

usr.bin/beep/beep.1
50 ↗(On Diff #97676)

I don't have the authority to say for sure, but I think we should use space to separate the numbers from the units here and below.

pstef added inline comments.
usr.bin/beep/beep.1
61 ↗(On Diff #97678)

Typo, inclusively.

This revision is now accepted and ready to land.Oct 29 2021, 5:48 PM
This revision now requires review to proceed.Oct 29 2021, 5:51 PM

Are we good to go for the initial version of beep(1) ? Please accept!

This revision is now accepted and ready to land.Nov 3 2021, 10:31 PM

LGTM, a couple of minor comments on the man page that you can consider

usr.bin/beep/beep.1
31 ↗(On Diff #97680)

play a beep (as you use in the text below) reads nicer to me -- "play beep on soundcard" or even just "play a beep sound"?

44 ↗(On Diff #97680)

s/to the/on the/ maybe