Fixes 2532 throttle trigger priority#2534
Merged
Dunbaratu merged 7 commits intoKSP-KOS:developfrom Jun 16, 2019
Merged
Conversation
Added a case in BreakExecution() to protect itself if it gets called while there are no program contexts existing yet, and thus nothing for it *to* break. (So it doesn't crash if you call it while the CPU is powered down.)
(The difference is that DROPPRIORITY only lets you drop to whatever the level was that called you, not all the way to Normal if you weren't called from Normal.)
That's how it used to work, when it was called ALLOWINTERRUPT(), but the decision to only let it drop one level of prioirty means the docs had to be changed to match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2532
Essentially the earlier InterruptPriority system still wasn't fully implemented properly, and still had some messy unnecessarily clunky code in ContinueExecution() that was trying to handle the different types of trigger with special cases (instead of just using the priority level numerically and ignoring all else).
This fix does the following:
1 - Adds a new higher level priority - level 30 - for use with cooked control locks. They now enjoy the highest priority and will interrupt anything other than another cooked control lock.
2 - Removes some ugly code from CPU.ContinueExecution() that pre-dated the concept of InterruptPriority. (It tried to accomplish some of the same ideas but in a more complex special case kind of way). That code was actually interfering with interrupt priorities working properly. (It had to be removed in order to make priority level 30 actually succeed at interrupting level 20.)
3 - Some of the removed code mentioned above was present to attempt to support GUI callbacks interrupting other GUI callbacks just in case a GUI callback fired off something that lasts a long time so it would't freeze the rest of the GUI while it runs. (Normally a trigger doesn't interrupt a trigger of the same priority so this was meant to be a special case just for GUI callbacks). In testing I found out that this had never been working anyway, and even with that messy code there, GUI callbacks still wouldn't interrupt other GUI callbacks so the reason for that was moot anyway. From a backward-compatibilitly point of view it didn't have to be supported.
4 - Just in case someone does want to use a design pattern where one GUI callback lasts a long time and lets other GUI callbacks interrupt it, I provied a new built-in function,
DROPPRIORITYthat can be executed from inside a trigger to drop the priority back down to what it was prior to the interruption. Thus the trigger can say "go ahead and re-enable interruptions and allow myself to get interrupted".(The example program written in the docs shows a case of how things differ when DROPPRIORITY is present.)
I struggle with whether or not bullet point (4) should be described well in the docs, or should be hidden so only people who read the long CPU_hardware page see it. (so it can't be used by someone who's never looked at the section on priorities and doesn't understand the consequences).