Skip to content

Instantly share code, notes, and snippets.

@phoddie
Created November 22, 2021 07:29
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save phoddie/481128bd0a2c01aa2ee7dffcf33b4ea7 to your computer and use it in GitHub Desktop.
Save phoddie/481128bd0a2c01aa2ee7dffcf33b4ea7 to your computer and use it in GitHub Desktop.

The LoRa module is a blend of the IO Class Pattern and the Peripheral Class Pattern from Ecma-419. The following notes explain where it doesn't quite conform.

constructor

The options object should put each IO into its own sub-object rather than merging them all together. Something like this:

export const LoRa_Heltec_Wifi_Lora_32_v2_pins = {
	lora: {
		io: device.io.SPI,
		clock: 5,
		in: 19,
		out: 27,
		select: 18,
		active: 0,
		hz: 9_000_000,
		port: 1,
		mode: 0,
	},
	reset: {
		io: device.io.Digital,
		pin: 14,
		mode: device.io.Digital.Output
	}
	interrupt: {
		io: device.io.Digital,
		pin: 26,
		mode: device.io.Digital.Input
	}
};

The constructor is passed in as part of each IO. That avoids having the LoRa module depend on a specific IO constructor. For example, the reset and interrupt pins might be on a GPIO expander rather than a built-in pin. The module will work unchanged in that case.

(Note interrupt instead of int to following naming conventions.

The GT911 touch sensor driver is a good example of this. It uses i2c to talk to the sensor and also has a separate interrupt line. Notice that it doesn't import any IO as it just uses the constructors passed to it.

configure instead of set

The Peripheral Class Pattern defines configure to do example what you've done with set.

There is no get. We discussed it but didn't come to a conclusion. Your get is fine. Note that it returns {...this.#options} where I think you intend {...this.#runtimeOptions}.

close

Ecma-419 requires that it be safe to call close multiple times.

The LoRa close always calls sleep. If it is closed twice, the second time it will throw an exception on sleep because SPI has been closed.

read

This looks pretty reasonable. You aren't required to accept an input buffer argument. The main reason to do that is for high frequency reads to avoid GC thrashing. But LoRa isn't that high frequency, so that won't happen anyway. I suggest LoRa do what UDP does - always return the full packet in a new ArrayBuffer. That simplifies the code.

Strictly speaking, if no packet is available, read should return undefined.

write

This one is a little subtle. Ideally the behavior of write shouldn't change depending on whether onWritable is provided. When write is called, it will write if it can. If not, it throws. If the caller wants to know when it is safe to write, it provides onWritable to be notified.

I think if you do that, then every write can be async. (This is exactly how write works for serial, for example.)

Write is also an all-or-nothing operation. If it cannot accept all the bytes, it throws and accepts none of them.

The size argument is probably unnecessary. If the caller wants to pass a view on a buffer, they can use something like subarray to create a view on the buffer.

details

It should be enough to check for undefined properties with

if (undefined !== options.int)

rather than

typeof options.int !== "undefined"

I prefer the former because it is faster (no string comparison).

Nice use of BigInt in setFrequency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment