Refactor location and format of all available commands

[pingevt]

Looking at the server setup, Commands are never setup in any kind of config. And they appear to be added in 2 places. Commands for running launchpad from the CLI, and commands from with Launchpad itself. I think the two places makes sense. But the server now needs to know about the commands, so we need to config them or something.

considering these 2 places, do we need two types of commands, CLI and internal to launchpad(?)?

CC: @benjaminbojko

[benjaminbojko]

A central spot, possibly a static array or enum in the Command class makes the most sense to me.

That way we could use a simple filter/reduce function to configure yargs (to include/exclude certain commands for startup), and the server package could expose all of those commands to the API via a simple iteration or lookup. It might even be nice if the server doesn't store any list of commands internally and simply does a check for whether a command exists once it receives a request, or iterates over all available commands to create routes (e.g. for REST).

[benjaminbojko]

PS: renamed the ticket to make it more actionable/clear what this is about

[pingevt]

Agreed on the iterable part! I like the idea of creating things on the fly, BUT do we ever see in the future where we may want to limit specific routes for the server or specific transports? I guess we can add more to the config to define that... but just curious.

[pingevt]

OK - didn't take a full stab at this yet, but this could be bigger than smaller. Not sure if this should be done before we try to launch the server, or if we should launch the server, then figure out what we need in this central commands thing.

[benjaminbojko]

It felt like this could be a fairly targeted incision, but I hear you on it feeling like it might have implications. It doesn't seem like it would limit functionality wildly.

Maybe a first step is just moving the StartupCommands enum from ./packages/launchpad/index.js to the CommandCenter class and leaving the rest as is?

And in regards to limiting specific routes: Yes, that's sort of what I meant! We could use the set of available commands and only allow routes that are valid commands, and return 404 or 401 accordingly.

[pingevt]

So... should Command Center be treated as a singleton?

[benjaminbojko]

Hmm that's a good question. Technically yes, there should only be one instance.

The only other actual singleton right now is the LogManager class. I wonder if we should make Launchpad a singleton and then expose the CommandCenter instance there via a getter. I worry about opening the door too much to wires getting crossed amongst packages, so maybe forcing things to go through Core will keep things at least somewhat centralized.

FWIW I'm still passing the _logger instance to each package as a constructor argument. I believe the singleton is only used in testing and other scenarios.

What do you think?

[pingevt]

I don't know.... these are great questions!

So, which is first/primary? The command center or Launchpad?

use case: Adding a user. This "could be" a command, but launchpad-core doesn't need to do anything with it and it is a command line only command. So its a waste to load launchpad-core to run that type of command.

• OR -

Launch-pad core is considered central and EVERYTHING needs to run through launchpad-core. Even adding a new user.

...

I'm now wondering too... could the command center be used for the dashboard as well? So its now a whole new new Config-like object (but its only defined in code - maybe we can enable/disable commands from config but not create new commands). Anything and everything can build out how it needs to based on that Command Center.

The more I'm typing i'm thinking thats the way to go... Let me branch off the server branch and play a bit.

[pingevt]

@benjaminbojko

So this is what is kind of bouncing around in my head. Then in Launchpad core, we get rid of the public methods and just have a simple run(Command) method which handles the actual logic of each command and how it works. The transport parts need to be worked out a bit as we implement them fully but the idea is certain items under each transport could potentially be modified in config.

export class LpCommands {
	/** @type {string} */
	static START = 'start';

	/** @type {string} */
	static STOP = 'stop';

	/** @type {string} */
	static CONTENT = 'content';

	/** @type {string} */
	static MONITOR = 'monitor';

	/** @type {string} */
	static SCAFFOLD = 'scaffold';

	/** @type {string} */
	static STATUS = 'status';
}

export const CommandCenter = {
	start: {
		id: LpCommands.START,
		description: "Starts launchpad by updating content and starting apps.",
		alias: [],
		http: {
			enabled: true,
			path: "/start",
		},
		ws: {
			enabled: true,
			type: "start",
		},
		osc: {
			enabled: true,
			address: "btngrp1/btn1",
			value: [0, 0],
		}
	},
	stop: {
		id: LpCommands.STOP,
		description: "Stops and kills any existing PM2 instance.",
		alias: [],
		http: {
			enabled: true,
			path: "/stop",
		},
		ws: {
			enabled: true,
			type: "stop",
		},
		osc: {
			enabled: true,
			address: "btngrp1/btn2",
			value: [0, 0],
		}
	},
	content: {
		id: LpCommands.CONTENT,
		description: "Only download content. Parses your config as `config.content || config`.",
		alias: ["update-content"],
		http: {
			enabled: true,
			path: "/content",
		},
		ws: {
			enabled: true,
			type: "content",
		},
		osc: {
			enabled: true,
			address: "btngrp1/btn3",
			value: [0, 0],
		}
	},
	monitor: {
		id: LpCommands.MONITOR,
		description: "Only start apps. Parses your config as `config.monitor || config`.",
		alias: ["start-apps"],
		http: {
			enabled: true,
			path: "/monitor",
		},
		ws: {
			enabled: true,
			type: "start-apps",
		},
		osc: {
			enabled: true,
			address: "btngrp1/btn4",
			value: [0, 0],
		}
	},
	scaffold: {
		id: LpCommands.SCAFFOLD,
		description: "Configures the current PC for exhibit environments (with admin prompt).",
		alias: [],
		http: {
			enabled: false,
			path: "/scaffold",
		},
		ws: {
			enabled: false,
			type: "scaffold",
		},
		osc: {
			enabled: false,
			address: "",
			value: "",
		}
	},
	status: {
		id: LpCommands.STATUS,
		description: "Provides a basic Overview of the Status of Launchpad.",
		alias: [],
		http: {
			enabled: true,
			path: "/status",
		},
		ws: {
			enabled: true,
			type: "status",
		},
		osc: {
			enabled: true,
			address: "btngrp1/btn6",
			value: [0, 0],
		}
	}
};

[pingevt]

OK - naming might be bad... that class maybe should be CmmandCenterConfig or something since it should replace the existing CommandCenter that is in core.

[benjaminbojko]

Thanks for pushing on this and giving this more thought! Super helpful to have something to look at and discuss.

I'd like to hear more about what you were thinking about configuring each protocol on a per-command basis. I was thinking that protocol-specific paths, message would either be a config in the transport layer or hard-coded. If the transport classes convert whatever messages they receive to these standardized commands then, then we can add/modify those transports whenever and don't need to keep updating this core spec. They could even be their own npm modules eventually for easer extensibility (e.g. specific project APIs). Feels cleaner to me to have that all decoupled.

Sidebar: The more I think about it, the more I question whether the HTTP server should actually be always on. It feels like that should actually just be one of the transport layers. That's a separate issue though #18 #14 :)

For the code, I was hoping to leverage what we already have more:

export class Command {
  /**
   This class already exists in `command-center.js`:
   https://github.com/bluecadet/launchpad/blob/main/packages/launchpad/lib/command-center.js#L121-L164
   
   The `name` property could be renamed to `id` if needed.

   We could switch the callbacks on each command to events,
   so that the `LaunchpadCore` class subscribes to those instead
   of dictating what the callback is within the command itself.
   That would allow other classes to also digest those events.
   */
}

// Then we have a class, enum or frozen object (`Object.freeze({...})`) that lists out all available commands
export class Commands {
  static START = new CommandSpec({id: 'start', /* ... */});
  static STOP = // ...
}

Let me know what you think!

[pingevt]

sidebar first: yea, in config the server as a whole should be able to be enabled or disabled as well as the individual transports. Its a little weird bc theres overlap with http and websockets but I think its handled. If you're talking about the enabled in each command, i was thinking an easy option would be to allow/disallow each command on each transport. So for example OSC, you can only trigger content download, but can't stop launchpad

Yea the more I played with this yesterday, the more I got confused and annoyed.. ha! I'm not in love with what I was thinking anymore. I was thinking the Command class was getting implemented too late in the process so was thinking we need config at the beginning. Add rather than refactor. And a little bit here I just didn't understand your thoughts and direction for the Command class.

One reason I was thinking it needs to be a separate config thing, is that a lot should be hard coded, but what if we need ???... I don't know at the moment, but not trying to lock in too much. Just the nature of OSC does seem like we are going to need to be able to config the address and values. And the nature of http and websockets, we can easily hard code those endpoints.

So:

• Should this be "solved" before the whole server is ready? Might be good to get the server almost ready so we understand what it really needs.
• This might be something worth having a convo over and planning properly