From 4241d70be76a1c1fac2388c6f18a0c6c2980749a Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 14:07:17 -0700 Subject: [PATCH 01/14] Add already-referenced CLI option struct members --- cli.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cli.go b/cli.go index c704058..7205617 100644 --- a/cli.go +++ b/cli.go @@ -14,10 +14,14 @@ import ( ) type CLIOpts struct { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Add the required variable for CLI parsing here + setcap bool + sinkName string + loadInput bool + loadOutput bool + unload bool + threshold int + list bool + checkUpdate bool } func parseCLIOpts() CLIOpts { From 827721907167eb5c442bbbafbbad6dc73b336377 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 14:29:00 -0700 Subject: [PATCH 02/14] Implement cleanupExit function in cli.go The TODO comment said to "unload" the librrnoise library, but from looking at the usage of `ctx.librrnoise`, the only thing that needs to be done is removing that file using `removeLib()`, since the library is only *loaded* by PulseAudio/PipeWire when a corresponding module is loaded. --- cli.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cli.go b/cli.go index 7205617..1de3072 100644 --- a/cli.go +++ b/cli.go @@ -176,8 +176,6 @@ func doCLI(opt CLIOpts, config *config, librnnoise string) { } func cleanupExit(librnnoise string, exitCode int) { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Unloads the specified library then exit with the specified exit code + removeLib(librnnoise) + os.Exit(exitCode) } From b636b18ca5eea9741b72fe5ebd8231ed3a088f41 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 14:49:07 -0700 Subject: [PATCH 03/14] Implement handling of `setcap` CLI option --- cli.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cli.go b/cli.go index 1de3072..6d9f23c 100644 --- a/cli.go +++ b/cli.go @@ -60,11 +60,13 @@ func doCLI(opt CLIOpts, config *config, librnnoise string) { cleanupExit(librnnoise, 0) } - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Check for setcap parameter and handle it. - // The function makeBinarySetcapped will be called. + if opt.setcap { + exitCode := 0 + if err := makeBinarySetcapped(); err != nil { + exitCode = 1 + } + cleanupExit(librnnoise, exitCode) + } paClient, err := pulseaudio.NewClient() if err != nil { From fe9cba5ccaa84a2726b932bcb9bff28282f6d804 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 15:02:18 -0700 Subject: [PATCH 04/14] Implement "verbose" CLI option for logging If the `-v` option is specified on the CLI, NoiseTorch will print logs to stderr. If not, log output will be silently discarded. Handling of this is done in `cli.go` instead of in `main.go` where the TODO was listed for it. This makes more sense, as handling of CLI options should be done as much as possible within the `doCLI` function, not in `main`. --- cli.go | 14 ++++++++++---- main.go | 5 ----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cli.go b/cli.go index 6d9f23c..b76acc0 100644 --- a/cli.go +++ b/cli.go @@ -6,6 +6,8 @@ package main import ( "flag" "fmt" + "io/ioutil" + "log" "os" "strings" @@ -14,6 +16,7 @@ import ( ) type CLIOpts struct { + verbose bool setcap bool sinkName string loadInput bool @@ -26,10 +29,7 @@ type CLIOpts struct { func parseCLIOpts() CLIOpts { var opt CLIOpts - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Add a parameter for logs + flag.BoolVar(&opt.verbose, "v", false, "Verbose output (print logs to stderr)") flag.BoolVar(&opt.setcap, "setcap", false, "for internal use only") flag.StringVar(&opt.sinkName, "s", "", "Use the specified source/sink device ID") flag.BoolVar(&opt.loadInput, "i", false, "Load supressor for input. If no source device ID is specified the default pulse audio source is used.") @@ -44,6 +44,12 @@ func parseCLIOpts() CLIOpts { } func doCLI(opt CLIOpts, config *config, librnnoise string) { + if opt.verbose { + log.SetOutput(os.Stderr) + } else { + log.SetOutput(ioutil.Discard) + } + if opt.checkUpdate { latestRelease, err := getLatestRelease() if err == nil { diff --git a/main.go b/main.go index 89b89d3..2308b38 100644 --- a/main.go +++ b/main.go @@ -57,11 +57,6 @@ func main() { opt := parseCLIOpts() - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Check for the log parameter and decide what - // will the std output be log.Printf("Application starting. Version: %s (%s)\n", version, distribution) log.Printf("CAP_SYS_RESOURCE: %t\n", hasCapSysResource(getCurrentCaps())) From f936c945c199e51cc5bf409a7681e984c5f16da0 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 16:39:00 -0700 Subject: [PATCH 05/14] Implement `loadModule()` function Uses the NoiseTorch context's `paClient` and calls the `LoadModule()` function to load a module. If an error is returned, it creates a new error view (hopefully correctly, I just copied the usage of `makeErrorView()` from a different function). --- module.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/module.go b/module.go index b0c764f..e5729b5 100644 --- a/module.go +++ b/module.go @@ -184,13 +184,12 @@ func loadSupressor(ctx *ntcontext, inp *device, out *device) error { return nil } -func loadModule(ctx *ntcontext, module, args string) (uint32, error) { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Loads a module using pcClient. - // Then checks if it loaded correctly. - // Will display an error if any. +func loadModule(ctx *ntcontext, module string, args string) (uint32, error) { + index, err := ctx.paClient.LoadModule(module, args) + if err != nil { + ctx.views.Push(makeErrorView(ctx, err.Error())) + } + return index, err } func loadPipeWireInput(ctx *ntcontext, inp *device) error { From 084e5c0a7a5e7421b7b0c0de490176060f405e0b Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 18:38:10 -0700 Subject: [PATCH 06/14] Implement `updatefn()` --- ui.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ui.go b/ui.go index 7317d66..6bb9173 100644 --- a/ui.go +++ b/ui.go @@ -58,11 +58,12 @@ var lightBlue = color.RGBA{173, 216, 230, 255} const notice = "NoiseTorch Next Gen (stylized NoiseTorch-ng) is a continuation of the NoiseTorch\nproject after it was abandoned by its original author. Please do not confuse\nboth programs. You may convey modified versions of this program under its name." +// updatefn() is run whenever the window needs to be re-drawn. +// This is usually only after user input. func updatefn(ctx *ntcontext, w *nucular.Window) { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Must display the view that's on top of the view stack + // Call view at the top of the stack + ctx.views.Peek()(ctx, w) +} func mainView(ctx *ntcontext, w *nucular.Window) { From d67883202e4c7f5be36315000e2ef2468280c6e5 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 18:38:58 -0700 Subject: [PATCH 07/14] Implement error views I copied the layout code from the setcap view, since I noticed while using NoiseTorch that the error view has the same layout, just different text. I also decided to make the title red to make the UI a little more differentiable. --- ui.go | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/ui.go b/ui.go index 6bb9173..505b0d9 100644 --- a/ui.go +++ b/ui.go @@ -437,22 +437,40 @@ func capabilitiesView(ctx *ntcontext, w *nucular.Window) { func makeErrorView(ctx *ntcontext, errorMsg string) ViewFunc { return func(ctx *ntcontext, w *nucular.Window) { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Indicates that there is an error and display "errorMsg" - // The user is allowed to continue using the program + w.Row(15).Dynamic(1) + w.LabelColored("Error", "CB", color.RGBA{255, 0, 0, 255}); + w.Row(15).Dynamic(1) + // this should probably use LabelWrap to avoid cutting off long + // messages, but when I tried, it made the text not appear at all + w.Label(errorMsg, "CB"); + + // whitespace + w.Row(40).Dynamic(1) + + w.Row(25).Dynamic(1) + if w.ButtonText("OK") { + ctx.views.Pop() + } } } func makeFatalErrorView(ctx *ntcontext, errorMsg string) ViewFunc { return func(ctx *ntcontext, w *nucular.Window) { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Indicates that there is a fatal error and display "errorMsg" - // The user is not allowed to continue using the program - // The program will exit + w.Row(15).Dynamic(1) + w.LabelColored("Fatal Error", "CB", color.RGBA{255, 0, 0, 255}); + w.Row(15).Dynamic(1) + // this should probably use LabelWrap to avoid cutting off long + // messages, but when I tried, it made the text not appear at all + w.Label(errorMsg, "CB"); + + // whitespace + w.Row(40).Dynamic(1) + + w.Row(25).Dynamic(1) + if w.ButtonText("Exit") { + ctx.views.Pop() + os.Exit(1) + } } } From 0fdefa8eb366ac8ef1bd4d44c50aa0204f29883b Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 18:55:56 -0700 Subject: [PATCH 08/14] Implement confirm view Uses the same layout as the error views, only with 2 buttons instead of one. I changed the title text color to yellow to differentiate it from the message text. Other than that, it behaves the same as before. --- ui.go | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/ui.go b/ui.go index 505b0d9..cf92362 100644 --- a/ui.go +++ b/ui.go @@ -474,14 +474,27 @@ func makeFatalErrorView(ctx *ntcontext, errorMsg string) ViewFunc { } } -func makeConfirmView(ctx *ntcontext, title, text, ?, ? string, ?, ? func()) ViewFunc { - return func(ctx *ntcontext, ? *nucular.Window) { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Confirmation dialog - // You should deduce some of the variable names its implementation - // from looking at its usage in the code +func makeConfirmView(ctx *ntcontext, title, text, proceedText, cancelText string, proceedFunc, cancelFunc func()) ViewFunc { + return func(ctx *ntcontext, w *nucular.Window) { + w.Row(15).Dynamic(1) + w.LabelColored(title, "CB", color.RGBA{255, 255, 0, 255}); + w.Row(15).Dynamic(1) + // this should probably use LabelWrap to avoid cutting off long + // messages, but when I tried, it made the text not appear at all + w.Label(text, "CB"); + + // whitespace + w.Row(40).Dynamic(1) + + w.Row(25).Dynamic(2) + if w.ButtonText(cancelText) { + ctx.views.Pop() + cancelFunc() + } + if w.ButtonText(proceedText) { + ctx.views.Pop() + proceedFunc() + } } } From ec831565304a754d5d2a6ba6665b62d0ce33ba14 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 19:03:18 -0700 Subject: [PATCH 09/14] Add `appName` variable to context This allows parts of the program outside the `main` function to use the app name without hard-coding a name. Will be useful to display the name in error messages. --- main.go | 1 + ui.go | 1 + 2 files changed, 2 insertions(+) diff --git a/main.go b/main.go index 2308b38..3a55044 100644 --- a/main.go +++ b/main.go @@ -65,6 +65,7 @@ func main() { defer removeLib(rnnoisefile) ctx := ntcontext{} + ctx.appName = appName ctx.config = readConfig() ctx.librnnoise = rnnoisefile diff --git a/ui.go b/ui.go index cf92362..566dfeb 100644 --- a/ui.go +++ b/ui.go @@ -34,6 +34,7 @@ type ntcontext struct { views *ViewStack serverInfo audioserverinfo virtualDeviceInUse bool + appName string } //TODO pull some of these strucs out of UI, they don't belong here From a73eae869d812d99538de6166f0750dda77663b3 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Sun, 28 Aug 2022 19:04:45 -0700 Subject: [PATCH 10/14] Implement `resetUI` function Since I don't have an outdated version of PipeWire, I couldn't reproduce the original error message, so I just guessed as to what it should be. --- ui.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/ui.go b/ui.go index 566dfeb..1d51b3f 100644 --- a/ui.go +++ b/ui.go @@ -500,19 +500,16 @@ func makeConfirmView(ctx *ntcontext, title, text, proceedText, cancelText string } func resetUI(ctx *ntcontext) { - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Create a new viewstack and push the main view + ctx.views = NewViewStack() + ctx.views.Push(mainView) if !ctx.haveCapabilities { ctx.views.Push(capabilitiesView) } - - // TODO: CODE REMOVED - // MUST BE WRITTEN FROM SCRATCH WITHOUT LOOKING AT THE ORIGINAL CODE - // Description: - // Check the server info for an outdated pipewire - // Then display and error message if it's too old (0.3.28 required) + if ctx.serverInfo.outdatedPipeWire { + ctx.views.Push(makeFatalErrorView(ctx, + fmt.Sprintf("Detected outdated version of PipeWire. %s requires PipeWire v0.3.28 or newer.", + ctx.appName))) + } } From 25f9da99e8bc90ffce71c0a7d8d3a8fd6e8404a3 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Mon, 29 Aug 2022 09:25:51 -0700 Subject: [PATCH 11/14] Use predefined UI color names Now uses orange as the color for the title on confirmation views since a yellow color was not already defined. --- ui.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui.go b/ui.go index 1d51b3f..638adfc 100644 --- a/ui.go +++ b/ui.go @@ -439,7 +439,7 @@ func capabilitiesView(ctx *ntcontext, w *nucular.Window) { func makeErrorView(ctx *ntcontext, errorMsg string) ViewFunc { return func(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) - w.LabelColored("Error", "CB", color.RGBA{255, 0, 0, 255}); + w.LabelColored("Error", "CB", red); w.Row(15).Dynamic(1) // this should probably use LabelWrap to avoid cutting off long // messages, but when I tried, it made the text not appear at all @@ -458,7 +458,7 @@ func makeErrorView(ctx *ntcontext, errorMsg string) ViewFunc { func makeFatalErrorView(ctx *ntcontext, errorMsg string) ViewFunc { return func(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) - w.LabelColored("Fatal Error", "CB", color.RGBA{255, 0, 0, 255}); + w.LabelColored("Fatal Error", "CB", red); w.Row(15).Dynamic(1) // this should probably use LabelWrap to avoid cutting off long // messages, but when I tried, it made the text not appear at all @@ -478,7 +478,7 @@ func makeFatalErrorView(ctx *ntcontext, errorMsg string) ViewFunc { func makeConfirmView(ctx *ntcontext, title, text, proceedText, cancelText string, proceedFunc, cancelFunc func()) ViewFunc { return func(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) - w.LabelColored(title, "CB", color.RGBA{255, 255, 0, 255}); + w.LabelColored(title, "CB", orange); w.Row(15).Dynamic(1) // this should probably use LabelWrap to avoid cutting off long // messages, but when I tried, it made the text not appear at all From 2a7ec816538f125ed63107b578a5ec582b540770 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Mon, 29 Aug 2022 16:56:55 -0700 Subject: [PATCH 12/14] Wrap text in views with possibly long messages Uses the LabelWrap component instead of the Label component, which provides text wrapping capability. Instead of a small row for text and a large whitespace row, the text row is now the height of both combined. This is required as when text wraps, it needs extra space below for the text to flow onto. When the text doesn't wrap, the extra height still displays as whitespace, making the design consistent with before this change. --- ui.go | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/ui.go b/ui.go index 638adfc..2319624 100644 --- a/ui.go +++ b/ui.go @@ -407,15 +407,14 @@ func connectView(ctx *ntcontext, w *nucular.Window) { func capabilitiesView(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) w.Label("This program does not have the capabilities to function properly.", "CB") - w.Row(15).Dynamic(1) - w.Label("We require CAP_SYS_RESOURCE. If that doesn't mean anything to you, don't worry. I'll fix it for you.", "CB") + w.Row(100).Dynamic(1) + w.LabelWrap("We require CAP_SYS_RESOURCE. If that doesn't mean anything to you, don't worry. I'll fix it for you.") if ctx.capsMismatch { w.Row(15).Dynamic(1) w.LabelColored("Warning: File has CAP_SYS_RESOURCE but our process doesn't.", "CB", orange) w.Row(15).Dynamic(1) w.LabelColored("Check if your filesystem has nosuid set or check the troubleshooting page.", "CB", orange) } - w.Row(40).Dynamic(1) w.Row(25).Dynamic(1) if w.ButtonText("Grant capability (requires root)") { err := pkexecSetcapSelf() @@ -440,13 +439,8 @@ func makeErrorView(ctx *ntcontext, errorMsg string) ViewFunc { return func(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) w.LabelColored("Error", "CB", red); - w.Row(15).Dynamic(1) - // this should probably use LabelWrap to avoid cutting off long - // messages, but when I tried, it made the text not appear at all - w.Label(errorMsg, "CB"); - - // whitespace - w.Row(40).Dynamic(1) + w.Row(100).Dynamic(1) + w.LabelWrap(errorMsg); w.Row(25).Dynamic(1) if w.ButtonText("OK") { @@ -459,13 +453,8 @@ func makeFatalErrorView(ctx *ntcontext, errorMsg string) ViewFunc { return func(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) w.LabelColored("Fatal Error", "CB", red); - w.Row(15).Dynamic(1) - // this should probably use LabelWrap to avoid cutting off long - // messages, but when I tried, it made the text not appear at all - w.Label(errorMsg, "CB"); - - // whitespace - w.Row(40).Dynamic(1) + w.Row(100).Dynamic(1) + w.LabelWrap(errorMsg); w.Row(25).Dynamic(1) if w.ButtonText("Exit") { @@ -479,13 +468,8 @@ func makeConfirmView(ctx *ntcontext, title, text, proceedText, cancelText string return func(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) w.LabelColored(title, "CB", orange); - w.Row(15).Dynamic(1) - // this should probably use LabelWrap to avoid cutting off long - // messages, but when I tried, it made the text not appear at all - w.Label(text, "CB"); - - // whitespace - w.Row(40).Dynamic(1) + w.Row(100).Dynamic(1) + w.LabelWrap(text); w.Row(25).Dynamic(2) if w.ButtonText(cancelText) { From bf599bd9bcebb73ea75e5ede934b251edd2d765b Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Fri, 2 Sep 2022 20:47:57 -0700 Subject: [PATCH 13/14] Print detected PipeWire version in error message --- ui.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui.go b/ui.go index 2319624..1edfea8 100644 --- a/ui.go +++ b/ui.go @@ -493,7 +493,7 @@ func resetUI(ctx *ntcontext) { if ctx.serverInfo.outdatedPipeWire { ctx.views.Push(makeFatalErrorView(ctx, - fmt.Sprintf("Detected outdated version of PipeWire. %s requires PipeWire v0.3.28 or newer.", - ctx.appName))) + fmt.Sprintf("Detected outdated version of PipeWire (v%d.%d.%d). %s requires PipeWire v0.3.28 or newer.", + ctx.serverInfo.major, ctx.serverInfo.minor, ctx.serverInfo.patch, ctx.appName))) } } From 7461486661a9d9cdbf37f26719a0aac9375f1a48 Mon Sep 17 00:00:00 2001 From: Kian Kasad Date: Fri, 2 Sep 2022 21:50:04 -0700 Subject: [PATCH 14/14] Add short title to capabilities view Adds a shorter main title to the capabilities view which can be centered and not wrap. The previous title is displayed below, but is left-aligned and can wrap. Also changes "I'll fix it for you" to "We'll fix it..." because the previous sentence says "We require..." and not "I require..." --- ui.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ui.go b/ui.go index 1edfea8..f62b7d1 100644 --- a/ui.go +++ b/ui.go @@ -406,9 +406,11 @@ func connectView(ctx *ntcontext, w *nucular.Window) { func capabilitiesView(ctx *ntcontext, w *nucular.Window) { w.Row(15).Dynamic(1) - w.Label("This program does not have the capabilities to function properly.", "CB") - w.Row(100).Dynamic(1) - w.LabelWrap("We require CAP_SYS_RESOURCE. If that doesn't mean anything to you, don't worry. I'll fix it for you.") + w.LabelColored("Missing capabilities", "CB", orange) + w.Row(50).Dynamic(1) + w.LabelWrap("This program does not have the capabilities to function properly.") + w.Row(60).Dynamic(1) + w.LabelWrap("We require CAP_SYS_RESOURCE. If that doesn't mean anything to you, don't worry. We'll fix it for you.") if ctx.capsMismatch { w.Row(15).Dynamic(1) w.LabelColored("Warning: File has CAP_SYS_RESOURCE but our process doesn't.", "CB", orange)