os.pullEvent and os.startTimer not playing nicely

I am having trouble with a CC program I'm using to control a Big Reactors reactor and turbine. The code still lacks a whole lot but once I get this basic part down I should be fine.

Here is the code: http://pastebin.com/2kjSbjHC

The part I'm having trouble with is the function manual(). I can't figure out a way to make the timer work to refresh the information on the screen once per second. It will start out at the proper refresh rate, but it seems every time an event is fired (namely monitor_touch) the refresh rate speeds up, almost double. It gets to the point where if you click the monitor too many times, the program will crash and the monitor will simply freeze until you terminate it.

What is the proper way to go about doing this?

Thank you!

Check which timer ID you're getting:

if event == "timer" and par1 == timer then

You also should only start the next timer at the end of that section of the if tree, not at the beginning of the while loop.

You could also try to cancel the timers when the monitor_touch events are received to avoid having multiple timers running.

os.cancelTimer(timer)

That's possible to do, yes, but it's poor design, comparatively. The best solution is to check which timer event it is and to only start new timers where appropriate.

Wait, when did cancelling become a thing?

Bomb Bloke, on 04 January 2015 - 09:31 AM said:

Wait, when did cancelling become a thing?

I did also wonder about that :D/> Apparently there also is os.cancelAlarm() now.

Thank you all so much for that. I figured it had something to do with multiple timers running but I didn't know how to fix it. Not knowing there was an os.cancelTimer() function I tried setting timer to nil, but of course that doesn't actually cancel the timer.

I have since moved the timer initialization to the proper place and placed a check in the if statement, I'll see how that works.

Thanks again!

I would just like to post that this solution did work as intended: pastebin.com/LCqBu8jP

Thank you again! Man, the speed at which you can get great help on these forums. I'm impressed!

Lyqyd, on 04 January 2015 - 06:47 AM said:

That's possible to do, yes, but it's poor design, comparatively. The best solution is to check which timer event it is and to only start new timers where appropriate.

I've had problems with using only the check method. Depending on how often you trigger the other events you can end up with so many timer events queued up that it causes a problem.

The best solution I've found is using both the check and canceling the invalid timers.

Cycomantis, on 04 January 2015 - 11:06 PM said:

-snip

The best way as lyqyd said is to only create new timers would it is actually needed. This will cause no "invalid timers" from ever being created in the first place. A way around this is to use a while loop to check for the timer.

while true do
  mytimer = os.startTimer(1)
  while true do
	event = {os.pullEvent()}
	if event[2] == mytimer then
	  --do stuff
	  break
	else
	  -- do other stuff
	end
  end
end

Using that even if there was a invalid timer somewhere in it. It would not break the loop or start another one. It would continue to check for the same timer until it received it, while allowing you to still do other things in as many else/elseif's as you wish.

edit: This would only create a invalid timer if you were to break the loop in a else statement, but it would not break the loop from working as it should.

valithor, on 04 January 2015 - 11:15 PM said:

–snip

It probably depends on the application. In the following code I use the same structure just rather then two while loops I use a while and a repeat. Due to the way I am utilizing the timers if I don't also include the cancelTimer functions I end up with some nasty bugs. If you can suggest a better way without I am all open to it. Always up for learning something new.

while true do
    printScreen()
    local timer = os.startTimer(nTimer)
    repeat
        local e,p1,p2,p3,msg = os.pullEvent()
        if e == "modem_message" and p2 == channelNum and msg == "harvest" then
            os.cancelTimer(timer)
            nTimer = 40
        elseif e == "modem_message" and p2 == channelNum then
            os.cancelTimer(timer)
            nTimer = 7
            currentTree,currentLumber,totalTree,totalLumber,mode,sapplings,fuel = string.match(msg, "(%w+),(%w+),(%w+),(%w+),(%w+),(%w+),(%w+)")
            if mode == "BoneMeal" then mode = "Bone Meal" end
        elseif e == "timer" and p1 == timer then
            mode = "Offline"
        elseif e == "monitor_resize" and p1 == monSide then
            setupMon()
            drawScreen()
            printScreen()
        end
    until (e == "modem_message" and p2 == channelNum) or (e == "timer" and p1 == timer)
end

Looks to me like that would function exactly the same without the cancelTimer calls (assuming the rest of the code you're presumably running this lot in parallel with is likewise making sure to correctly check incoming events).

It's possible that making those calls speeds things up over just allowing the timers to expire naturally, though I'm doubting it. Dunno. Personally, instead of cancelling the timers and waiting for a new one to be created on the next iteration, I'd just create a new timer on the spot.

By the way, any particular reason you're extracting your message contents out of a string, instead of just sending them in table form and unpacking from there? Eg, say you originally sent the message like this:

rednet.send(123, {currentTree,currentLumber,totalTree,totalLumber,mode,sapplings,fuel} )

You'd then pull the content out like this:

currentTree,currentLumber,totalTree,totalLumber,mode,sapplings,fuel = unpack(msg)

It'd be easier again to just leave the values in the table.