Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actually support interrupts #46

Closed
AeroStun opened this issue Apr 29, 2020 · 7 comments · Fixed by #63
Closed

Actually support interrupts #46

AeroStun opened this issue Apr 29, 2020 · 7 comments · Fixed by #63
Assignees
Labels
high-priority Needs to be done before all others currently in queue sprint-5 User stories & bugs for TBD on sprint 5

Comments

@AeroStun
Copy link
Contributor

AeroStun commented Apr 29, 2020

Right now interrupts are never fired
This needs to change; people often rely on them, and even smartcar_shield has a snippet relying on them.

This basically means have suspendable threads
On Windows use SuspendThread
On Un*x, attempt to use

  • POSIX pthread_suspend (which basically nobody implemented anywhere)
  • or pthread_suspend_np (OpenBSD and FreeBSD implemented that)
  • or a similar trick to what dotnetcore does for macOS and Linux
    Some ideas and info can be dug from this SO answer and its comments

Acceptance criteria: Interrupts are fired when the user expects them to, and they do not cause deadlocks.

@AeroStun AeroStun added enhancement New feature or request help wanted Extra attention is needed labels Apr 29, 2020
@AeroStun AeroStun self-assigned this Apr 29, 2020
@platisd
Copy link
Collaborator

platisd commented Apr 29, 2020

You could implement an observer pattern that unblocks each "interrupt thread" depending on the user's configuration. For instance, you could have the user insert a specially formatted comment on how often they would like their thread triggered.

//*~* 20ms
attachInterrupt(digitalPinToInterrupt(2), tick, RISING);

Just a first thought from the top of my head.

@AeroStun
Copy link
Contributor Author

Wouldn't that only apply to LOW and HIGH modes?
My understanding is that FALLING, RISING and CHANGE are actual events, so the ISR should only fire when these events happen (which of course we can detect since we capture all IO).

@platisd
Copy link
Collaborator

platisd commented Apr 29, 2020

I haven't really understood how you were planning to "visualize" this, but I imagine you would want the user to somehow set a frequency that "pulses" (and therefore trigger these events) are arriving or to be able trigger things manually (e.g. pressing a switch). This could be done somehow via a parsable comment in the code or a separate menu/config somewhere else in the emulation environment.

Btw, ultimately, all interrupts are CHANGE interrupts. If the new pin state is HIGH then it's a RISING event otherwise a FALLING.

@AeroStun
Copy link
Contributor Author

Well, let's take this basic example (as bad as it looks, at least it is simple):

void setup(){
  attachInterrupt(digitalPinToInterrupt(1), +[](){ Serial.println("Button pressed!"); }, RISING);
}

void loop(){}

Assuming a push button is correctly wired to GPIO1, to us expected behavior would be to have Button pressed\r\n be printed onto the console (or whatever other UART device is configured) as soon as possible
In our case, we might implement a push button as a UI button on the window that also has the camera view, which the user would press to see their desired effects (or not) on the screen/console

@AeroStun
Copy link
Contributor Author

Regarding the polling frequency, the game engine's milliseconds per tick are the bottleneck, since it is the only possible source of events, and all interrupts are checked at the end of every tick.
And even if we decide to put another "real-time" device with its own thread to talk to the board, we would still be capturing all its IO, hence we can immediately invoke any needed ISR

Our only problem is guaranteeing that the board thread won't be suspended in a syscall that locks system mutexes, since that would quickly cause a deadlock and hang the whole process (and that's obviously not what the user wants)

@platisd
Copy link
Collaborator

platisd commented Apr 29, 2020

Hmmm, don't understand the context fully so I can't add much more but good luck nonetheless! 😛

@AeroStun
Copy link
Contributor Author

Thanks, we'll need it; enjoy seeing the result whenever I manage to get it stable across all 3 major platforms and compilers

@AeroStun AeroStun added sprint-5 User stories & bugs for TBD on sprint 5 high-priority Needs to be done before all others currently in queue and removed enhancement New feature or request help wanted Extra attention is needed labels May 8, 2020
@AeroStun AeroStun linked a pull request May 16, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority Needs to be done before all others currently in queue sprint-5 User stories & bugs for TBD on sprint 5
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants