Additional sleep methods with array of pins as parameters ?

[mfalkvidd]

@nca78 I'm more interested in how it would be used by sketch developers than the internal library design.

How would I use the new feature from my sketch?

[Nca78]

@mfalkvidd said in Additional sleep methods with array of pins as parameters ?:

@nca78 I'm more interested in how it would be used by sketch developers than the internal library design.

How would I use the new feature from my sketch?

A bit like how it's done in the script I posted here:
https://forum.mysensors.org/topic/6961/nrf5-bluetooth-action/1514

You can trigger on any pin you want and know which pin triggered interrupt:

  if (interrupt_cause == PIN_BUTTON1) {
    // if door status changed, we send door message
    if (door_status != last_sent_value) {
      sendDoorStatus();
    }
  }
  else if (interrupt_cause == PIN_BUTTON2) {
    if (millis() < last_button2_event || (millis() - last_button2_event > 100)) {
      last_button2_event = millis();
      blinkityBlink(2, 3); // not so useful, just for testing :)
    }
  }
  else {  // end of sleeping period, we send battery level
    sendBatteryStatus();
  }

I can add any pin, at the moment I only have a hall sensor and a button, but I will also wake up on interrupts from light sensor, 2 different interrupts from accelerometer etc.
Pin change allows the same thing on atmega328.

[mfalkvidd]

So there would be a global variable called interrupt_cause? If that's the case, it should probably be renamed to latest_interrupt_cause since it may be updated at any time.

Would there be a return value of sleep() that tells whether the user should check this global variable, or how would they know to check it?

[Nca78]

@mfalkvidd said in Additional sleep methods with array of pins as parameters ?:

So there would be a global variable called interrupt_cause? If that's the case, it should probably be renamed to latest_interrupt_cause since it may be updated at any time.

Would there be a return value of sleep() that tells whether the user should check this global variable, or how would they know to check it?

No that's just in my code for nrf51 that is just some kind of "proof of concept" that pin sense works with MySensors and that it solves the hardware bug creating a 1ma power consumption with other methods. Of course all of that must be wrapped into much more user friendly methods if included in MySensors.
I think a "mySleepResult" structure like in my previous message (https://forum.mysensors.org/topic/9234/additional-sleep-methods-with-array-of-pins-as-parameters/8) is a better idea, with all information about the sleep summed up in one place, with some additional helper functions to check if a specific pin has changed (and in which direction) or not. If we don't do that it will be too complicated for the "non-programmer" user to retrieve information.

For user the basic process to use it would be :

• I declare the pins I want to watch (in theory as many pins as wanted)
• I call the "advancedSleep" (or whatever name we give it) method
• I get a simple return value (similar to now) telling me if there was an error, if it's the end of sleep duration or give me the (first) pin that triggered the wake up
• If I want to get more detailed information (direction of pin change, check multiple pins, duration of sleep to restart sleeping only for remaining time in original delay, ...) then I can check the global instance of the structure (MySleepResult) or call the helper methods.

[NeverDie]

@nca78 said in Additional sleep methods with array of pins as parameters ?:

@mfalkvidd said in Additional sleep methods with array of pins as parameters ?:

So there would be a global variable called interrupt_cause? If that's the case, it should probably be renamed to latest_interrupt_cause since it may be updated at any time.

Would there be a return value of sleep() that tells whether the user should check this global variable, or how would they know to check it?

No that's just in my code for nrf51 that is just some kind of "proof of concept" that pin sense works with MySensors and that it solves the hardware bug creating a 1ma power consumption with other methods. Of course all of that must be wrapped into much more user friendly methods if included in MySensors.
I think a "mySleepResult" structure like in my previous message (https://forum.mysensors.org/topic/9234/additional-sleep-methods-with-array-of-pins-as-parameters/8) is a better idea, with all information about the sleep summed up in one place, with some additional helper functions to check if a specific pin has changed (and in which direction) or not. If we don't do that it will be too complicated for the "non-programmer" user to retrieve information.

For user the basic process to use it would be :

• I declare the pins I want to watch (in theory as many pins as wanted)
• I call the "advancedSleep" (or whatever name we give it) method
• I get a simple return value (similar to now) telling me if there was an error, if it's the end of sleep duration or give me the (first) pin that triggered the wake up
• If I want to get more detailed information (direction of pin change, check multiple pins, duration of sleep to restart sleeping only for remaining time in original delay, ...) then I can check the global instance of the structure (MySleepResult) or call the helper methods.

I'm glad you're working on this!

For possible inspiration, there is precedent for calling a function/method for each pin that you want to set as an interrupt. For instance, for regular arduino's, the call might be:

  pciSetup(7);
  pciSetup(8);
  pciSetup(9);
  pciSetup(A0);

as illustrated by the sketch at: https://playground.arduino.cc/Main/PinChangeInterrupt

That said, I'm entirely neutral about how it gets done.

[Nca78]

@neverdie thank you for the link I didn't remember about that, I will have a look.

[alexsh1]

@mfalkvidd said

myAttachInterrupt(digitalPinToInterrupt(pin2), CHANGE));

Sorry to chip in, but what's myAttachInterrupt function please?
Do you also have to do:

pinMode(pin2, INPUT);
myAttachInterrupt(digitalPinToInterrupt(pin2), CHANGE));

[mfalkvidd]

@alexsh1 chipping in is great :) I don’t know what would be in it, I was just outlining how using the new feature could look like from a sketch developer’s perspective.

I don’t think sketch developer should need to worry about setting the pin to input, except maybe if they want to activate a pullup/pulldown feature (if the mcu supports it).

[alexsh1]

@mfalkvidd OK, I was concerned it was something already implemented and I did not know about it.
I'd rather leave it as is. I know the idea is to avoid interrupt handlers, but we need to re-think and re-write the sketch code so that we only use volatile variables for true/false statements.

[Yveaux]

@mfalkvidd said in Additional sleep methods with array of pins as parameters ?:

I don’t think sketch developer should need to worry about setting the pin to input,

Yes, they have to set it to input. There's no pin configuration in the (AVR) sleep code: https://github.com/mysensors/MySensors/blob/development/hal/architecture/AVR/MyHwAVR.cpp#L153

[Nca78]

@yveaux said in Additional sleep methods with array of pins as parameters ?:

Yes, they have to set it to input. There's no pin configuration in the (AVR) sleep code: https://github.com/mysensors/MySensors/blob/development/hal/architecture/AVR/MyHwAVR.cpp#L153

But atmega pins default as inputs so it's not really mandatory with the default IC.

[Yveaux]

@nca78 said in Additional sleep methods with array of pins as parameters ?:

@yveaux said in Additional sleep methods with array of pins as parameters ?:

Yes, they have to set it to input. There's no pin configuration in the (AVR) sleep code: https://github.com/mysensors/MySensors/blob/development/hal/architecture/AVR/MyHwAVR.cpp#L153

But atmega pins default as inputs so it's not really mandatory with the default IC.

I like to make things explicit, especially if you decide to port to another architecture later on

[Nca78]

@yveaux said in Additional sleep methods with array of pins as parameters ?:

I like to make things explicit, especially if you decide to port to another architecture layer on

I completely agree with you. Just stating that @mfalkvidd is not wrong when saying at the moment it's not necessary to declare pins as inputs

To go back to the main subject should I make a first version of this code (in post 9) to have a clearer basis for future discussions ? Or am I going a wrong way ?

[NeverDie]

Anything is always better than nothing.

[mfalkvidd]

@yveaux clarification: imho, if support for many interrupts is added in a future version of MySensors (which is what we're discussing in this thread), sketch developers should not need to manually call pinMode.