toitlang/jaguar

Crash when an envelope can't be found

yudistrange opened this issue · 11 comments

I have an ESP32-S3 chipset. I am trying to install jaguar on the chipset but failing with the following error.

❯ jag flash -p /dev/ttyACM1 -c ESP32-S3

Downloading ESP32-S3 firmware from https://github.com/toitlang/envelopes/releases/download/v2.0.0-alpha.160/firmware-ESP32-S3.envelope.gz ...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xb2e6a8]

goroutine 1 [running]:
github.com/toitlang/jaguar/cmd/jag/commands.downloadGzipped({0xe089d8?, 0xc000199c50?}, {0xc000042a10?, 0xccf6a2?}, {0xc00007f3e0, 0x52})
	/home/runner/work/jaguar/jaguar/cmd/jag/commands/envelope.go:44 +0x48
github.com/toitlang/jaguar/cmd/jag/commands.downloadPublishedFirmware({0xe089d8, 0xc000199c50}, {0xc000040cc0, 0x10}, {0x7ffe28ed88b5, 0x8})
	/home/runner/work/jaguar/jaguar/cmd/jag/commands/envelope.go:136 +0x2ad
github.com/toitlang/jaguar/cmd/jag/commands.GetCachedFirmwareEnvelopePath({0xe089d8, 0xc000199c50}, {0xc000040cc0, 0x10}, {0x7ffe28ed88b5, 0x8})
	/home/runner/work/jaguar/jaguar/cmd/jag/commands/envelope.go:27 +0x105
github.com/toitlang/jaguar/cmd/jag/commands.FlashCmd.func1(0xc0001ecf08, {0xc0001ce840, 0x0, 0xccf6d2?})
	/home/runner/work/jaguar/jaguar/cmd/jag/commands/flash.go:97 +0x6e5
github.com/spf13/cobra.(*Command).execute(0xc0001ecf08, {0xc0001ce800, 0x4, 0x4})
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0xc0001e2608)
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/spf13/cobra.(*Command).ExecuteContext(0xcd1106?, {0xe089d8?, 0xc000199c50?})
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1032 +0x47
main.main()
	/home/runner/work/jaguar/jaguar/cmd/jag/main.go:34 +0x18b

Try with jag flash -p /dev/ttyACM1 -c esp32s3

I will try to harden the Jaguar executable.

FYI: you can see all available envelopes here: https://github.com/toitlang/envelopes/releases

Ah! Because I was using the capitalized name, it was going to the wrong link. Thanks!

Reopening: Jaguar shouldn't just crash when there is a bad chip.

Fair.
Also I had copied the chip name from the error message that I got when I tried flashing without providing a chipset. So that may need changing (unless the fix makes jaguar handle that)

The error message probably comes from the esptool, and would be hard to change :(

Confirmed. The error message is from the esptool.

Hmm.. I guess we can have a supported list of chips here

And fail gracefully when the supplied chipset name doesn't match. What do you think?

I think I will special case the most common chip names and mistakes and otherwise invite the user to check that the chip name is correct.
Something like:

		if err := downloadPublishedFirmware(ctx, version, model); err != nil {
			switch model {
			case "esp32", "esp32s3", "esp32c3", "esp32s2":
				fmt.Printf("Failed to download firmware: %v\n", err)
			case "ESP32", "ESP32S3", "ESP32C3", "ESP32S2":
				fmt.Printf("Chip model names must be lowercase. Please try again with 'esp32', 'esp32s3', 'esp32c3', or 'esp32s2'.\n")
			case "ESP32-S2", "ESP32-S3", "ESP32-C3":
				fmt.Printf("Chip model names must be lowercase without dashes. Please try again with 'esp32s2', 'esp32s3', or 'esp32c3'.\n")
			default:
				fmt.Printf("Failed to download firmware: %v\n", err)
				fmt.Printf("Make sure the model name is correct. You can find supported models at\n")
				fmt.Printf("https://github.com/toitlang/envelopes/releases/tag/%s.\n", version)
			}

Why not lower case the supplied chipset name before checking against the supported list anyway?
The names with - can fail as you suggested.

This is unrelated, but I see other envelopes as well such as esp32s3-no-ble.
When are they used? Can a user supply those as the chipset name? Or are they for internal use only?

Why not lower case the supplied chipset name before checking against the supported list anyway? The names with - can fail as you suggested.

Agreed. I'm considering that.

This is unrelated, but I see other envelopes as well such as esp32s3-no-ble. When are they used? Can a user supply those as the chipset name? Or are they for internal use only?

They can be used, and sometimes should be used. For example, if the ESP32 has SPIRAM, or octo-SPIRAM. See the README of the repository.

Since BLE uses memory even when not in use, it's beneficial to use that envelope if possible. Simply use --chip esp32s3-no-ble.