Unable to simply toggle an LED on pico

[bartmichu]

I've noticed that simple LED toggle is not possible on pico when using the IO module with this pattern: led.write(!led.read())

Code example

import Digital from 'embedded:io/digital';

const led = new Digital({
  pin: 25,
  mode: Digital.Output
});

const button = new Digital({
  pin: 22,
  mode: Digital.Input,
  edge: Digital.Falling,
  onReadable () {
    led.write(!led.read());
  }
});

More info
Example code fails due to these checks:

if (!digital->isInput)
		xsUnknownError("unimplemented");

Is it intentional? I'm a bit surprised as it used to work well on ESP platforms the last time I was playing with Moddable.

[phoddie]

You are calling read on the LED. The LED is an output, so it is write-only. The check you indicate above is trying to tell you that:

moddable/modules/io/digital/pico/digitalbank.c

Lines 319 to 325 in 2420f19

	void xs_digitalbank_read(xsMachine *the)
	{
	Digital digital = xsmcGetHostDataValidate(xsThis, (void *)&xsDigitalBankHooks);
	uint32_t result = 0;
	if (!digital->isInput)
	xsUnknownError("unimplemented");

The ESP32 implementation has an identical check.

Are you certain this exact code worked as-is on ESP32?

[bartmichu]

According to my git commits I was using this on NodeMCU and Moddable One about two years ago. I'll try to dig out my Moddable One board and test it properly - as I should've done before opening this issue, to be honest.

[bartmichu]

OK @phoddie so I have double checked now and this exact code works as I expected on ESP8266 (NodeMCU).

Also, does reading a digital PIN have some side effects? Read-only mode is obvious, but write-only not necessarily (to somebody who knows nothing about electronics that is).

[phoddie]

Thanks for the additional details. In the original report you wrote "ESP platforms." I interpreted that as ESP32, but actually it was ambiguous. ;) ESP8266 does not perform the check, so that explains why you didn't get an exception. Interesting that read returns a useful value for you there. Still, I'm sure that's not a reliable behavior across devices.

Embarrassingly, I left myself a note to add the error check in the ESP8266 implementation:

moddable/modules/io/digital/esp/digitalbank.c

Lines 437 to 440 in 2420f19

	//@@ verify read is allowed
	uint32_t modDigitalBankRead(Digital digital)
	{
	uint32_t result = GPIO_REG_READ(GPIO_IN_ADDRESS) & digital->pins;

I'll fill that in now so there is a consistent behavior across silicon architectures.

Thank you for the report!

[bartmichu]

Could you please clarify one thing for me.

I was not sure about the meaning of "unimplemented", but now when you have changed the message, am I right assuming that it will not be implemented at all?

Is it something unique to the IO module? Or a bad idea in general - the traditional pins/digital module allows reading an output as well - and you would advise not using it like that?

[tve]

Yup... I also find the pin implementation in io.Digital too restrictive, see EcmaTC53/spec#35
Most pins follow a clean input or output pattern, but many don't and are 'abused' in many ways. Reading an output pin makes a lot of sense, not just to toggle but also to read the state of an open drain pin, for example. It would be an interesting exercise to implement a OneWire driver using io.Digital...