search

tinygo-org / drivers

TinyGo drivers for sensors, displays, wireless adaptors, and other devices that use I2C, SPI, GPIO, ADC, and UART interfaces.
https://tinygo.org
BSD 3-Clause "New" or "Revised" License
587 stars 180 forks source link

wifinina: add generated strings, improved debugging system and messages #560

Closed deadprogram closed 1 year ago

deadprogram commented 1 year ago

This PR adds generated strings, improved debugging system and improved error messages to the wifinina driver.

deadprogram commented 1 year ago

With this PR:

sendCmd: Start StartClientTCP(2D) 04 (3)
sendParam8: 0 lastParam: false 
sendParam8: 0 lastParam: true 
spiChipDeselect
waitForChipReady()
spiChipDeselect
wifinina error: TimeoutChipReady
deadprogram commented 1 year ago

The only drawback of this PR is that it does increase the resulting size by nearly 1.5k:

Before this PR:

tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/ntpclient/main.go
go: downloading golang.org/x/net v0.0.0-20210614182718-04defd469f4e
go: downloading golang.org/x/text v0.3.6
   code    data     bss |   flash     ram
 134492    2468    6056 |  136960    8524
5a9025262f3decd7b331a8b8f0c54d75  ./build/test.hex
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/udpstation/main.go
   code    data     bss |   flash     ram
 134492    2464    6064 |  136956    8528
adc31f618547adcbfe5d2eeb2cb274bd  ./build/test.hex
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/tcpclient/main.go
   code    data     bss |   flash     ram
 134700    2476    6056 |  137176    8532
526bb08ba389a3b6b12d9ffa88e93e50  ./build/test.hex
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/webclient/main.go
   code    data     bss |   flash     ram
 135024    2476    6056 |  137500    8532
3c00af7c526b1f4d52566a56cf797eee  ./build/test.hex

With this PR

tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/ntpclient/main.go
go: downloading golang.org/x/net v0.0.0-20210614182718-04defd469f4e
go: downloading golang.org/x/text v0.3.6
   code    data     bss |   flash     ram
 136096    2468    6056 |  138564    8524
cd7b727cf628b948394e9d8050c7aa0d  ./build/test.hex
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/udpstation/main.go
   code    data     bss |   flash     ram
 136096    2464    6064 |  138560    8528
374065f1ff7d224cb2ccdb5119cf1891  ./build/test.hex
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/tcpclient/main.go
   code    data     bss |   flash     ram
 136[308](https://github.com/tinygo-org/drivers/actions/runs/4841230648/jobs/8627335753#step:8:309)    2476    6056 |  138784    8532
2ec94c900d92e688cdd9dbf6176ca245  ./build/test.hex
tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/webclient/main.go
   code    data     bss |   flash     ram
 136628    2476    6056 |  139104    8532
b7c636add26b4ade799ce248c3e50bb7  ./build/test.hex
aykevl commented 1 year ago

Oof, that's a big increase! Not sure about this. (I generally have less of a problem with binary size in drivers because they're easier to avoid / optimize, but this is still quite a lot!). Here is one alternative idea: log the error code and a link (possibly a shortlink) to a page where the error message can be looked up easily. That shouldn't result in a big code size increase, although it is still a bit less usable.

(I won't block this PR over the code size increase if you think it's worth it, just thinking of alternatives that might reduce the binary size somewhat).

deadprogram commented 1 year ago

OK, I just changed this to put most of the strings behind the wifidebug tag and still leave better error strings at cost of only 450 bytes.

Size before this PR:

$ tinygo build -size short -o test.hex -target pyportal -ldflags="-X main.ssid=rems -X main.pass=Salvador1" ./examples/wifinina/mqttsub/
   code    data     bss |   flash     ram
 180120    3036    8120 |  183156   11156

This PR without the wifidebug tag:

$ tinygo build -size short -o test.hex -target pyportal -ldflags="-X main.ssid=XXX -X main.pass=YYY" ./examples/wifinina/mqttsub/          
   code    data     bss |   flash     ram                                                                                                         
 180576    3036    8120 |  183612   11156

This PR with the wifidebug tag:

$ tinygo build -size short -o test.hex -target pyportal -ldflags="-X main.ssid=XXX -X main.pass=YYY" -tags=wifidebug ./examples/wifinina/mq
ttsub/                                                                                                                                            
   code    data     bss |   flash     ram                                                                                                         
 183048    3036    8120 |  186084   11156
deadprogram commented 1 year ago

Now that I have been testing https://github.com/tinygo-org/drivers/pull/537 I am going to close this PR. The error strings can be added in a separate PR once https://github.com/tinygo-org/drivers/pull/537 makes it in.

deadprogram commented 1 year ago

Reopening since #537 will not merge until TinyGo 0.29

deadprogram commented 1 year ago

Since this is now only 450 bytes size increase in normal (non-debug) mode, I am going to merge. We really do need it.