Loop not called if attachInterrupt(..LOW)

[ricardojlrufino]

Hi,
i'm running some examples of interrupt and I found strange behavior.

Based on the example (which works correctly in wokwi online)
https://wokwi.com/projects/318885209198035539

If I run the same example in Avr8js it will trigger the interrupt (e not call loop())
As the inputs are configured as PULLUP, was this supposed to happen?

I tried simulating the same behavior, removing the buttons from the example, and it still works correctly
https://wokwi.com/projects/328609280331612756

But no work in avr8js-electron-playground , and avr8js demo in my code (which is based on his..)
I wonder if I need to configure something else.

Reproduce:

• Copy this code into avr8js demo

// Reference: https://wokwi.com/projects/318885209198035539

#define inputPin 2 // which input pin is connected to button *
const int debounceDelay = 10; // milliseconds to wait until button input stable *
#define inputWildpin 3 // which input ping is connect to button NOT debounced

#define LED 12  // The pin the button-LED is connected to
#define LEDWild 13 // The pin used for second non-debounced connected LED


void setup() {
  // put your setup code here, to run once
  Serial.begin(115200); // initializes serial monitor for troubleshooting and learning
  Serial.println("Hi !");
  delay(100);
  
  pinMode(inputPin, INPUT_PULLUP); // make button pin mode input *
  digitalWrite(inputPin, HIGH);
  pinMode(LED, OUTPUT); // Declare the button-LED as an output
  
  
  pinMode(inputWildpin, INPUT_PULLUP); // make second button pin mode input
  digitalWrite(inputWildpin, HIGH);
  pinMode(LEDWild, OUTPUT); // Declare the button-LED as an output- second comparison LED


  attachInterrupt(digitalPinToInterrupt(inputWildpin), buttonPushed2, LOW); // assign interrupt to button, procedure to be run and set state to LOW when pushed
  attachInterrupt(digitalPinToInterrupt(inputPin), buttonPushed, LOW); // assign interrupt to button, procedure to be run and set state to LOW when pushed *

}

void loop() {
  // put your main code here, to run repeatedly:
  Serial.println("[loop] while waiting for button"); // normal program goes here
  delay(1000); // pause to read serial monitor output
}

void buttonPushed() {   // detect and debounce button push using assigned interrupt *
  if (debounce(inputPin)) { // calls debounce procedure *
    Serial.println("Debounce Button Pushed"); // shows button pushed on serial monitor
    digitalWrite(LED, HIGH); // Turn the button-LED on for feedback
    Serial.println("  something done when button is pushed"); // normal program code for action when button pressed
    Serial.println(""); // blank line for neatness
    delay(1000); // delay to read serial monitor and see LED
    digitalWrite(LED, LOW); // Turn the button-LED back off
  }
}

void buttonPushed2() {  // detect second button push using assigned interrupt
  Serial.print('x');delay(50);
  if (digitalRead(inputPin)) { // does NOT call debounce procedure
    Serial.println("@@ Non-debounced Button Pushed @@"); // shows button pushed on serial monitor
    digitalWrite(LEDWild, HIGH); // Turn the button-LED on for feedback
    Serial.println("  something done when non-debounced button is pushed"); // normal program code for action when button pressed
    Serial.println(""); // blank line for neatness
    delay(1000); // delay to read serial monitor and see LED
    digitalWrite(LEDWild, LOW); // Turn the button-LED back off
  }
}

// ***************************************
// **            Debounce pin           **
// **    Entire procedure required      **
// **     outside of setup or loop      **
// **           usually at end          **
// ***************************************
// debounce returns true if the switch in the given pin is closed and stable
boolean debounce(int pin)
{
  boolean state;
  boolean previousState;
  previousState = digitalRead(pin); // store switch state
  for (int counter = 0; counter < debounceDelay; counter++)
  {
    delay(1); // wait for 1 millisecond
    state = digitalRead(pin); // read the pin
    if ( state != previousState)
    {
      counter = 0; // reset the counter if the state changes
      previousState = state; // and save the current state
    }
  }
  // here when the switch state has been stable longer than the debounce period
  return state;
}

[ricardojlrufino]

Its a PULLUP problem...
this code must print:

[up] 1
[down] 1

void setup() {
  Serial.begin(115200);
  pinMode(2, INPUT_PULLUP);
  digitalWrite(2, HIGH);
  
  pinMode(3, INPUT_PULLUP);
  digitalWrite(3, HIGH);
 
  pinMode(13, OUTPUT);
  digitalWrite(13, HIGH);

  delay(1000); // Pause for 2 seconds
  Serial.print("[up] \n"); Serial.println(digitalRead(3));
  Serial.print("[down] \n"); Serial.println(digitalRead(2));
  
  delay(1000); // Pause for 2 seconds
  
}

void loop() {
  int up = digitalRead(3);
  int down = digitalRead(2);

  if (up == LOW) {;
    Serial.println("[up]");
    delay(200);
  };
  if (down == LOW) {
    Serial.println("[down]");
    delay(200);
  };
}

[ricardojlrufino]

this implicit PULLUP has same problem
If the pin is configured as an INPUT, digitalWrite() will enable (HIGH) or disable (LOW) the internal pullup on the input pin.

 pinMode(2, INPUT);
  digitalWrite(2, HIGH);
  
  pinMode(3, INPUT);
  digitalWrite(3, HIGH);

[ricardojlrufino]

Working online

[urish]

AVR8js does not include the logic to deal with pull-up resistors, so the implementation is up to the user of the library.

You can add a listener to the relevant GPIO port (by calling the addListener() method of the AVRIOPort class), and then check the state for each of the pins, and call setPin() to set the final input value of the pin. Something along the following lines:

for (let pin = 0; pin < 8; pin++) {
  if (port.pinState(pin) === PinState.InputPullUp) {
     // Now, if the button is pressed call port.setPin(pin, false);
     // Otherwise call port.setPin(pin, true);
  }
}

You can find a more complete example in the AVR8js Simon Game Sample. The code there assumes there's always a pull-up resistor on the pin (e.g. an external pull-up resistor), so the logic is hard coded:

for (const button of buttons) {
  const pinIndex = parseInt(button.getAttribute('pin'), 10);
  button.addEventListener('button-press', () => {
    if (runner) {
      runner.portD.setPin(pinIndex, false);
    }
  });
  button.addEventListener('button-release', () => {
    if (runner) {
      runner.portD.setPin(pinIndex, true);
    }
  });
}

If you need additional help with this, feel free to reopen / add new comments to this thread

[ricardojlrufino]

Hi @urish , thanks for the feedback

But this is a feature of the AVR at least 328p, according to the datasheet.
I don't think it would be interesting to include this functionality ?

[ricardojlrufino]

I rum this code in SimulIDE and PicSimLAB (simavr) and both implement this control...

void setup() {
  Serial.begin(115200);
  
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  
  digitalWrite(2, HIGH);
  digitalWrite(3, HIGH);

  delay(100); 
  Serial.print("[up]:"); Serial.println(digitalRead(3));
  Serial.print("[down]:"); Serial.println(digitalRead(2));
  
  delay(1000); 
  
}

void loop() {

}

[ricardojlrufino]

Online works....

But if i comment, in this example: https://stackblitz.com/edit/avr8js-simon-game-j4rxnx?file=index.ts , not works

for (const button of buttons) {
    const pinIndex = parseInt(button.getAttribute('pin'), 10);
    //runner.portD.setPin(pinIndex, true);
  }

[urish]

Thanks for the feedback, Ricardo!

It is a design choice to expose the building blocks to the users of the library and let them implement the pin functionality as they wish. AVR8js does let you know if the pull-up for a specific has been enabled or not, but it's up to you to decide how to act upon it. This way, we don't impose any limitations on what you can connect to the pins.

For instance, on Wokwi.com, the implementation takes into account external resistors as well, so you can also use an external pull-up or pull-down resistor. Also, a floating INPUT pin currently has a random state.

Other implementations can choose to also take the resistor values into account (and this might be the case in Wokwi once wokwi/wokwi-features#203 is done), so by adding a 1k resistor between a pulled-up pin and the ground, you'll get the pin to read 0, but adding a 1m resistor the pin will still read 1. For even more accuracy, the implementation can take the chip's internal temperature into account:

One may even integrate AVR8js with a complete SPICE simulation to create a more accurate model of the digital pins, adjusting the pin threshold voltage according to the supply voltage:

[ricardojlrufino]

I understand, I see that this logic is outside the CPU, my head is still a closed view that the CPU is just one thing...

So deciding how to implement this would be part of controlling peripherals. Something like it's right here?

In my case I "depend" that there is a button, to simulate the pulllup resistor. at wokwi.com, you don't have to. How did you do it there?

It's just that I still think that other people possibly people are going to have this problem, and maybe something like a flag in AVRIOPort , can help toggle this behavior on and off.

[urish]

Exactly!

Just like the SPI/TWI don't implement the on-wire protocol - they give you some handy callbacks, and you can decide if/how you support these protocols.

In Wokwi, the implementation looks at which devices are connected to each MCU pin. Then, whenever the state of any of these devices changes, it checks whether any of the devices have HIGH or LOW output. If there are none, it checks whether there are any pull up or pull down resistors (internal to the MCU or external). If none are found, it uses a random value (high or low). Once it determines the final digital value of the pin, it calls the setPin() method with this value.

If Wokwi had an analog simulation engine (similar to falstad), we'd probably model the pull-up resistor using a combination of a 47kΩ resistor (or similar value) + MOSFET that connects/disconnects the resistor according to the MCU pin configuration.

What would the suggested flag in AVRIOPort do? How do you imagine it would work?

[ricardojlrufino]

I don't know what happens, but online it works, without adding any external components
Just Open : https://wokwi.com/projects/new/arduino-uno
And run:

void setup() {
  Serial.begin(115200);
  
  pinMode(2, INPUT);
  digitalWrite(2, HIGH);

  delay(100); 

  if(digitalRead(2) == LOW) Serial.println("FAIL");
  else Serial.println("SUCCESS");
}

void loop() {

}

THIS PRINTS: SUCCESS !!

Now on : /avr8js/demo (http://localhost:1234)

This prints : FAIL

--

I believe the right behavior is:
IF DDR is set to INPUT, digitalWrite on PORTD/B must update PIND/B

Test using PORTs have sabe result

void setup() {
 
 Serial.begin(115200);
 
 DDRD = 0;

 PORTD |= _BV(PD2);

 if(PIND & (1 << PD2)) {  // if pin 2 high
   Serial.print("SUCCESS");
 }else{
   Serial.print("FAIL");  
 }

}

void loop() {

}

On SIMAVR, PIND changes !!
https://github.com/buserror/simavr/blob/master/simavr/sim/avr_ioport.c#L78

[ricardojlrufino]

For the previous print you can understand that it was changed on that place, but no, it was here:
PIND changes here

And here it is changed

[ricardojlrufino]

The great reason for existing this pull_up, is to do with digitalwrite (P, high), always keep high, maintaining consistency.
And the strategy of implementing this in hardware is to put the pull_up ...

But I think I understood the problem in the simulation. Is that if I set a high, but have a pressed button forcing Low. The final state should be "low" by the smallest "resistance". So only the external software can know if it exists or not "something connected";

On the other hand it can reverse logic:
When you set the pin for high
If in external software does not have anything connected (button): keep high (in case even if you do not have listeners in the port)
If in the external software has something connected on the pin, it decides whether it gets low (or high) based on some resistance rule.

[ricardojlrufino]

Thinkercad ignore internal pull_up

TEST SUMARY, return in:
arduino físco: SUCCESS
wokwi: SUCCESS
AVRSIM / PicSimlab (C): SUCCESS
Avr8JS: FAIL
Thinkercad: FAIL

[ricardojlrufino]

I tried, implement a test for this case.
But I do not know how to fulfill the fix ... kkkk
I'm still learning about AVR architecture

it('should enable internal pull-up if set HIGH to INPUT pin (issue #XXXX)', () => {
    const { program } = asmProgram(`
    ; register addresses
    _REPLACE DDRD, ${DDRD - 0x20}
    _REPLACE PIND, ${PIND - 0x20}
    _REPLACE PORTD, ${PORTD - 0x20}

    ; Setup
    ldi r23, 0x0
    out DDRD, r23

    ; Now toggle pin 2 with SBI
    sbi PORTD, 2

    break
  `);
    const cpu = new CPU(program);
    const portD = new AVRIOPort(cpu, portDConfig);
    const runner = new TestProgramRunner(cpu);

    const listener = jest.fn();
    portD.addListener(listener);

    // Setup: pins 2 as INPUT
    runner.runInstructions(2);
    expect(cpu.data[PIND]).toEqual(0x0);

    // Set: pins 2 as HIGH
    runner.runInstructions(1);
    expect(listener).toHaveBeenCalledWith(0x04, 0x0);
    expect(cpu.data[PORTD]).toEqual(0x4);
    expect(cpu.data[PIND]).toEqual(0x4);
  });

[urish]

Thinkercad ignore internal pull_up

This is surprising

But I do not know how to fulfill the fix ... kkkk

I think we can come up with an helper function or class that will attach to the AVRIOPort. That way, people who want to bring their own implementation (like Wokwi does), will keep using the existing API, but if someone just needs a basic implementation with support for buttons, they can use the helper function or class. Thoughts?

[ricardojlrufino]

I thought about putting a flag on AvrportConfig or AVRIOPort

where the user could decide whether or not to activate this functionality, so would not change the current behavior.

And Something like

[ricardojlrufino]

Pullup view: