Rev talk - Oct 2009 - [PATCH] fix on_timer event_callback for TimerWatcher

There was a timer_watcher event_callback defined for the
IOWatcher class in the timer_watcher.rb file.  I assume this is
a copy-and-paste error and a grep of the source shows no
"timer_watcher" methods ever being dispatched.  So change this
to allow the "on_timer" method to be overidden when given a
block.

Also added a spec to show it works.
---
 As usual, pushed up to git://yhbt.net/rev

 lib/rev/timer_watcher.rb   |    4 +-
 spec/timer_watcher_spec.rb |   49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 spec/timer_watcher_spec.rb

diff --git a/lib/rev/timer_watcher.rb b/lib/rev/timer_watcher.rb
index ecc5b52..8fe59ca 100644
--- a/lib/rev/timer_watcher.rb
+++ b/lib/rev/timer_watcher.rb
@@ -5,13 +5,13 @@
 #++
 
 module Rev
-  class IOWatcher
+  class TimerWatcher
     # The actual implementation of this class resides in the C extension
     # Here we metaprogram proper event_callbacks for the callback methods
     # These can take a block and store it to be called when the event
     # is actually fired.
     
     extend Meta
-    event_callback :timer_watcher
+    event_callback :on_timer
   end
 end
diff --git a/spec/timer_watcher_spec.rb b/spec/timer_watcher_spec.rb
new file mode 100644
index 0000000..dde994c
--- /dev/null
+++ b/spec/timer_watcher_spec.rb
@@ -0,0 +1,49 @@
+require File.dirname(__FILE__) + ''/../lib/rev''
+
+describe Rev::TimerWatcher do
+
+  it "can have the on_timer callback defined after creation" do
+    @watcher = Rev::TimerWatcher.new(0.01, true)
+    nr = ''0''
+    @watcher.on_timer { nr.succ! }.should == nil
+    @watcher.attach(Rev::Loop.default).should == @watcher
+    nr.should == ''0''
+    Rev::Loop.default.run_once
+    nr.should == ''1''
+  end
+
+  it "can be subclassed" do
+    class MyTimerWatcher < Rev::TimerWatcher
+      TMP = ''0''
+
+      def on_timer
+        TMP.succ!
+      end
+    end
+    @watcher = MyTimerWatcher.new(0.01, true)
+    @watcher.attach(Rev::Loop.default).should == @watcher
+    MyTimerWatcher::TMP.should == ''0''
+    Rev::Loop.default.run_once
+    MyTimerWatcher::TMP.should == ''1''
+  end
+
+  it "can have the on_timer callback redefined between runs" do
+    @watcher = Rev::TimerWatcher.new(0.01, true)
+    nr = ''0''
+    @watcher.on_timer { nr.succ! }.should == nil
+    @watcher.attach(Rev::Loop.default).should == @watcher
+    nr.should == ''0''
+    Rev::Loop.default.run_once
+    nr.should == ''1''
+    @watcher.detach
+    @watcher.on_timer { nr = :foo }.should == nil
+    @watcher.attach(Rev::Loop.default).should == @watcher
+    nr.should == ''1''
+    Rev::Loop.default.run_once
+    nr.should == :foo
+  end
+
+  after :each do
+    @watcher.detach if defined?(@watcher)
+  end
+end
-- 
Eric Wong

Eric Wong <normalperson at yhbt.net> wrote:
> There was a timer_watcher event_callback defined for the
> IOWatcher class in the timer_watcher.rb file.  I assume this is
> a copy-and-paste error and a grep of the source shows no
> "timer_watcher" methods ever being dispatched.  So change this
> to allow the "on_timer" method to be overidden when given a
> block.
> 
> Also added a spec to show it works.
> ---
Actually, I think the spec I added at the time was always
broken under 1.9...

Or I''m understanding Rev::Loop.run_once incorrectly.

I thought this was broken by my recent changes to make
Rev::Loop.run_once work better under 1.8, but reverting back[1] to the
original commit that introduced this spec reveals it was always broken
under 1.9.

I''ve pushed this out to git://yhbt.net/rev , but please take
this with a grain of salt since it''s probably too late for
me at night to be thinking clearly.

[1] - that means "git clean -f -d -x" in my working copy and any
      gem installations purged.   I even tried it on a clean box
      that''s never seen rev.git before.
>From ccb25d3f2a97c234ebf613757f2ee59179dda249 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson at yhbt.net>
Date: Wed, 2 Dec 2009 23:58:23 -0800
Subject: [PATCH] timer_watcher_spec: ensure interval has passed before running

Otherwise Rev::Loop.run_once may not have fired
---
 spec/timer_watcher_spec.rb |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/spec/timer_watcher_spec.rb b/spec/timer_watcher_spec.rb
index dde994c..b7305cd 100644
--- a/spec/timer_watcher_spec.rb
+++ b/spec/timer_watcher_spec.rb
@@ -2,12 +2,15 @@ require File.dirname(__FILE__) +
''/../lib/rev''
 
 describe Rev::TimerWatcher do
 
+  interval = 0.010
+
   it "can have the on_timer callback defined after creation" do
-    @watcher = Rev::TimerWatcher.new(0.01, true)
+    @watcher = Rev::TimerWatcher.new(interval, true)
     nr = ''0''
     @watcher.on_timer { nr.succ! }.should == nil
     @watcher.attach(Rev::Loop.default).should == @watcher
     nr.should == ''0''
+    sleep interval
     Rev::Loop.default.run_once
     nr.should == ''1''
   end
@@ -20,25 +23,28 @@ describe Rev::TimerWatcher do
         TMP.succ!
       end
     end
-    @watcher = MyTimerWatcher.new(0.01, true)
+    @watcher = MyTimerWatcher.new(interval, true)
     @watcher.attach(Rev::Loop.default).should == @watcher
     MyTimerWatcher::TMP.should == ''0''
+    sleep interval
     Rev::Loop.default.run_once
     MyTimerWatcher::TMP.should == ''1''
   end
 
   it "can have the on_timer callback redefined between runs" do
-    @watcher = Rev::TimerWatcher.new(0.01, true)
+    @watcher = Rev::TimerWatcher.new(interval, true)
     nr = ''0''
     @watcher.on_timer { nr.succ! }.should == nil
     @watcher.attach(Rev::Loop.default).should == @watcher
     nr.should == ''0''
+    sleep interval
     Rev::Loop.default.run_once
     nr.should == ''1''
     @watcher.detach
     @watcher.on_timer { nr = :foo }.should == nil
     @watcher.attach(Rev::Loop.default).should == @watcher
     nr.should == ''1''
+    sleep interval
     Rev::Loop.default.run_once
     nr.should == :foo
   end
-- 
Eric Wong

Sorry about the belated reply... I''ve been busy with other stuff,
especially
Reia :)

On Thu, Dec 3, 2009 at 1:22 AM, Eric Wong <normalperson at yhbt.net>
wrote:
> Eric Wong <normalperson at yhbt.net> wrote:
> > There was a timer_watcher event_callback defined for the
> > IOWatcher class in the timer_watcher.rb file.  I assume this is
> > a copy-and-paste error and a grep of the source shows no
> > "timer_watcher" methods ever being dispatched.  So change
this
> > to allow the "on_timer" method to be overidden when given a
> > block.
> >
> > Also added a spec to show it works.
> > ---
>
> Actually, I think the spec I added at the time was always
> broken under 1.9...
>
> Or I''m understanding Rev::Loop.run_once incorrectly.
>
Can you better clarify the behavior your were expecting with
Rev::Loop.run_once and timers?

Rev::Loop.run_once will perform a single pass through libev''s event
loop.
Your spec sleeps until the timer interval should''ve passed, and checks
if it
receives an event, which it does, at least when I run the spec.  So I''m
confused... what''s the problem?

> I thought this was broken by my recent changes to make
> Rev::Loop.run_once work better under 1.8, but reverting back[1] to the
> original commit that introduced this spec reveals it was always broken
> under 1.9.
>
> I''ve pushed this out to git://yhbt.net/rev , but please take
> this with a grain of salt since it''s probably too late for
> me at night to be thinking clearly.
>
Is this spec failing for you?

Anyway, merged and pushed...

-- 
Tony Arcieri
Medioh! A Kudelski Brand
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rev-talk/attachments/20091215/e51d5593/attachment.html>

Tony Arcieri <tony at medioh.com> wrote:
> On Thu, Dec 3, 2009 at 1:22 AM, Eric Wong <normalperson at yhbt.net>
wrote:
> > Eric Wong <normalperson at yhbt.net> wrote:
> > > There was a timer_watcher event_callback defined for the
> > > IOWatcher class in the timer_watcher.rb file.  I assume this is
> > > a copy-and-paste error and a grep of the source shows no
> > > "timer_watcher" methods ever being dispatched.  So
change this
> > > to allow the "on_timer" method to be overidden when
given a
> > > block.
> > >
> > > Also added a spec to show it works.
> > > ---
> >
> > Actually, I think the spec I added at the time was always
> > broken under 1.9...
> >
> > Or I''m understanding Rev::Loop.run_once incorrectly.
> 
> Can you better clarify the behavior your were expecting with
> Rev::Loop.run_once and timers?
> 
> Rev::Loop.run_once will perform a single pass through libev''s
event loop.
> Your spec sleeps until the timer interval should''ve passed, and
checks if it
> receives an event, which it does, at least when I run the spec.  So
I''m
> confused... what''s the problem?
I was mistaken and thought Rev::Loop.run_once would sleep the calling
thread on its own, but now that I think about it, it makes sense:
run_once may run in a different thread under 1.9.
> > I thought this was broken by my recent changes to make
> > Rev::Loop.run_once work better under 1.8, but reverting back[1] to the
> > original commit that introduced this spec reveals it was always broken
> > under 1.9.
> > I''ve pushed this out to git://yhbt.net/rev , but please take
> > this with a grain of salt since it''s probably too late for
> > me at night to be thinking clearly.
> 
> Is this spec failing for you?
> 
> Anyway, merged and pushed...
Without the patch, yes it was failing under 1.9.  All good now, thanks!

-- 
Eric Wong

On Tue, Dec 15, 2009 at 11:23 PM, Eric Wong <normalperson at yhbt.net>
wrote:
> I was mistaken and thought Rev::Loop.run_once would sleep the calling
> thread on its own, but now that I think about it, it makes sense:
> run_once may run in a different thread under 1.9.
>
run_once does not sleep.  It performs a single pass through the event loop
and dispatches any pending events, then immediately returns control back to
the caller once all pending events are processed.  It''s the
"non-blocking"
alternative to run.

> Without the patch, yes it was failing under 1.9.  All good now, thanks!
>
Great.  Hopefully I can do a new release soon.

-- 
Tony Arcieri
Medioh! A Kudelski Brand
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rev-talk/attachments/20091215/23bebc2c/attachment.html>

On Wed, Dec 16, 2009 at 10:16 PM, Roger Pack <rogerdpack at gmail.com>
wrote:
> Hooray for rev--the only evented ruby architecture with an
> on_write_complete LOL.
> -r
>
Honestly, this is the single most important event which is missing from the
EventMachine contract.

Perhaps I should''ve spent my time simply adding this callback to
EventMachine, but I preferred to build a true OO event API on top of Ruby.

-- 
Tony Arcieri
Medioh! A Kudelski Brand
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rev-talk/attachments/20091216/5efd5902/attachment.html>