Convert module to v3 format #4

peternewman · 2023-06-24T12:00:51Z

My first time doing this, and I've not tested against the software, but the expected commands appear in a Telnet window.

I figured this was a good way to start before looking at #2 / #3

index.js

dnmeid · 2023-06-24T12:53:23Z

index.js

-instance.prototype.destroy = function() {
-	var self = this;
+	incomingData(data) {
+		this.log('debug', data)


we now have log rotation, but nevertheless logging every piece of incoming data will pollute the log

I'm assuming/I don't believe there is much returned data (aside from possibly the version command which I've currently not implemented). I don't currently have a running older release with Telnet, but I was migrating the module to v3 before trying to add the current (beta) OSC support.

dnmeid · 2023-06-24T12:54:46Z

index.js


-	if (self.socket !== undefined) {
-		self.socket.destroy();
+		this.updateStatus(InstanceStatus.Ok, 'Received data')


You should not update the status on every incoming data, only when the status has to change.

I can't see a way to get the current status, do I have to double track it locally?

Yes, it depends on how the module makes use of the status and how it depends on the device or communication. We just want to make sure there is no flooding of status updates (possible DoS).

I've added some basic functionality, but it seems very backwards to me that companion-module-base couldn't deal with this under the hood using effectively the same code, to save each module author having to reinvent the wheel.

index.js

dnmeid · 2023-06-24T13:12:39Z

index.js

-		case 'pause':
-			cmd = 'pause';
-			break;
+		if (self.config.host) {


should check for existance and validiy of host and port

What exactly do you mean, won't the Regex handle that? Do you just mean not undefined, or do we need to check it's not an empty string too? I've added the port at least.

Yes and no. In actions or feedbacks the regex is just a visual hint, in the configuration you can't hit save if the regex doesn't match. This check in the config has not been there from the start so there may be configurations with illegal data. Also some users edit the configuration database by hand. It is always a good idea to do some extra checks. Minimum goal is that the module doesn't crash if it gets invalid data. You can go as complicated as you want. Best would be to do the regex again and do a status update and error logging if it fails.

So it turns out I'd not changed from this.REGEX_IP to Regex.IP and it was silently dropping all regex checks in the config, without throwing any errors during build, so that was a good fix at least.

Can you point me at another module that does this I could learn from, I've got something that actually works (but haven't committed it), after the obvious thing didn't work because a regex string needs foo whereas a regex literal needs /foo/ in JS!

Again it feels like we want to nick a chunk of:
https://github.com/bitfocus/companion/blob/master/webui/src/Components/TextInputField.jsx#L56-L66

Into companion-module-base if we're going to do this, and fetch the config from getConfigFields, so I just pass in the field name, and maybe the variable, and it gives me a boolean result, again rather than everyone reinventing the wheel badly/with different edge cases.

index.js

package.json

presets.js

peternewman · 2023-06-24T22:39:55Z

Thanks @dnmeid . As you can probably tell I'm not a JS expert (or at least not anything as modern as this) so thanks for the pointers. This is also my first time working on a companion module.

As a general thought, given there's a fairly finite number of common comms methods (e.g. TCP, UDP, Telnet, HTTP, OSC), are there, or would there be benefit in, some basic starter/template modules people could work from, without all the complexity of say the generic ones, and that might have less bad habits than working from other existing modules?

dnmeid · 2023-06-25T13:02:00Z

Don't worry if it is not perfect from the start. It will never be. After each release you will find things to improve.

As a general thought, given there's a fairly finite number of common comms methods (e.g. TCP, UDP, Telnet, HTTP, OSC), are there, or would there be benefit in, some basic starter/template modules people could work from, without all the complexity of say the generic ones, and that might have less bad habits than working from other existing modules?

That would be nice, but we need somebody to do it. It is allready a lot of work to code new features, find bugs, review PRs, update modules, give support to developers and users, write documentation...

…hecks

peternewman · 2023-07-14T17:50:22Z

That would be nice, but we need somebody to do it. It is allready a lot of work to code new features, find bugs, review PRs, update modules, give support to developers and users, write documentation...

Yeah fair point, in which case to turn that slightly on its head, as a developer of a new module I want to find a good HTTP example to work from say, without lots of trawling through code, its hard to know which modules talk via HTTP, so could there be a list of the top few HTTP based modules for me to go and look at? Obviously likewise for other high level protocols. Which is fairly minimal extra effort and might remove some of the developer support required. Plus then at least someone will be starting from a good base rather than the first one they come across which might have lots of issues...

package.json

index.js

Create dependabot.yml

Revert "Create dependabot.yml"

Revert "Revert "Create dependabot.yml""

Try and check dev dependencies too

Bumps [@companion-module/tools](https://github.com/bitfocus/companion-module-tools) from 0.5.2 to 1.2.1. - [Release notes](https://github.com/bitfocus/companion-module-tools/releases) - [Changelog](https://github.com/bitfocus/companion-module-tools/blob/main/CHANGELOG.md) - [Commits](bitfocus/companion-module-tools@v0.5.2...v1.2.1) --- updated-dependencies: - dependency-name: "@companion-module/tools" dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [@companion-module/base](https://github.com/bitfocus/companion-module-base) from 1.0.2 to 1.4.1. - [Release notes](https://github.com/bitfocus/companion-module-base/releases) - [Changelog](https://github.com/bitfocus/companion-module-base/blob/main/CHANGELOG.md) - [Commits](bitfocus/companion-module-base@v1.0.2...v1.4.1) --- updated-dependencies: - dependency-name: "@companion-module/base" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>

…ion-module/base-1.4.1 Bump @companion-module/base from 1.0.2 to 1.4.1

…ion-module/tools-1.2.1 Bump @companion-module/tools from 0.5.2 to 1.2.1

Merge my master changes back into v3

peternewman added 2 commits June 24, 2023 12:58

Convert module to v3 format

26e7b81

Format help Markdown file

daca624

dnmeid requested changes Jun 24, 2023

View reviewed changes

peternewman added 2 commits June 24, 2023 23:24

Fix a lot of the review comments

d7205ac

Fix a few more comments

28d4ee3

peternewman requested a review from dnmeid June 24, 2023 22:36

peternewman added 3 commits July 14, 2023 17:32

Validate that the host and port actually pass their respective regex

72144d4

Correct the regex constants and revert to some basic working config c…

f10165c

…hecks

Only send changed statuses

c56c62b

dnmeid requested changes Jul 16, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

index.js Outdated Show resolved Hide resolved

index.js Outdated Show resolved Hide resolved

peternewman and others added 17 commits July 16, 2023 20:55

Create dependabot.yml

fc7ba8e

Merge pull request #1 from peternewman/patch-1

afc400c

Create dependabot.yml

Revert "Create dependabot.yml"

73ca46b

Merge pull request #2 from peternewman/revert-1-patch-1

82f3445

Revert "Create dependabot.yml"

Revert "Revert "Create dependabot.yml""

afc8ad0

Merge pull request #3 from peternewman/revert-2-revert-1-patch-1

8957004

Revert "Revert "Create dependabot.yml""

Fix a few more review comments

e9bf8f7

Try and check dev dependencies too

66268da

Merge pull request #4 from peternewman/patch-1

c600579

Try and check dev dependencies too

Add some dependencies

feb02a6

Fix package.json syntax

765fd55

Revert to the simpler config for dependency checking

a9132d1

Merge pull request #6 from peternewman/dependabot/npm_and_yarn/compan…

93391a3

…ion-module/base-1.4.1 Bump @companion-module/base from 1.0.2 to 1.4.1

Merge pull request #5 from peternewman/dependabot/npm_and_yarn/compan…

efcd91c

…ion-module/tools-1.2.1 Bump @companion-module/tools from 0.5.2 to 1.2.1

Merge branch 'v3' into master

df5f0a7

Merge pull request #7 from peternewman/master

6046d1c

Merge my master changes back into v3

dnmeid approved these changes Jul 17, 2023

View reviewed changes

dnmeid merged commit 07fc900 into bitfocus:master Jul 17, 2023

peternewman mentioned this pull request Jul 18, 2023

Create dependabot.yml #5

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert module to v3 format #4

Convert module to v3 format #4

peternewman commented Jun 24, 2023

dnmeid Jun 24, 2023

peternewman Jun 24, 2023

dnmeid Jun 24, 2023

peternewman Jun 24, 2023

dnmeid Jun 25, 2023

peternewman Jul 14, 2023

dnmeid Jun 24, 2023

peternewman Jun 24, 2023 •

edited

Loading

dnmeid Jun 25, 2023 •

edited

Loading

peternewman Jul 14, 2023

peternewman commented Jun 24, 2023

dnmeid commented Jun 25, 2023

peternewman commented Jul 14, 2023

Convert module to v3 format #4

Convert module to v3 format #4

Conversation

peternewman commented Jun 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman Jun 24, 2023 • edited Loading

Choose a reason for hiding this comment

dnmeid Jun 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman commented Jun 24, 2023

dnmeid commented Jun 25, 2023

peternewman commented Jul 14, 2023

peternewman Jun 24, 2023 •

edited

Loading

dnmeid Jun 25, 2023 •

edited

Loading