Growl support for MacOSX package

Now isn't this just so cool 

I didn't get a chance to reply the day you sent it so please
excuse the 
delay.

Thanks a lot for both your contribution (which I like a lot)
and the 
"happy birthday" screen shot ... so cool 

I've said this to you already, but I am impressed by the
fact that you 
did this without asking a single question. Respect! 

Besides, your code is very well written and you respect our
coding and 
architecture conventions! Very good work!

more inline

Romain KUNTZ wrote:
> Hi Emil,
> 
> As you know I am working on adding the Growl
notification support 
> (http://growl.info/)
as a plugin for the MacOSX version of 
> sip-communicator.
> 
> With the enclosed files, an user can be notified via a
small window on 
> top of the screen when he receives or send a message.
This is very 
> useful when you use multiple desktops, or to follow a
discussion even if 
> the chat window is hidden (screenshot: 
> http:/
/www.rkuntz.org/sipco/sip-comm-growl.png ).
> 
> I enclosed several files to get the basic support of
this functionnality 
> (instead of a big patch which may not be easy to read):

yes, thanks, that's how I prefer them too.

> - growl.jar: a small library that needs to be put in
lib/

done. I only needed to recompile it so that it is 1.4
compatible. Please 
try it and let me know if it doesn't work.

> 
> - build.xml-20061207.patch: a patch to apply to
build.xml. It allows to 
> not build the bundle if it is not compiled on MacOSX,
and to get a 
> correct classpath for the "compile" target if
compilation is done on 
> macosx (Be careful, this patch considers that the
previous patch I sent 
> about sip-communicator.version has been applied)

A couple of comments here. there's no point in performing
all the 
classpath acrobatics. You can feel free to add
/System/Library/Java 
directly to the project classpath since this won't disturb
non-mac 
platforms (it would simply be ignored).

I've also modified your bundling target to produce the jar
in 
sc-bundles/os/macosx, in order to avoid having it included
in non-mac 
distros that are copying * from sc-bundles. I've also
updated the macosx 
target to get the growlnotification.jar file from
sc-bundles/os/macosx

> - a felix.client.run.properties specific for MacOSX: it
includes the 
> plugin and the growl lib. It must be put in
resources/install/macosx/
> I think this could be a good temporary solution. For
the long term, I 
> think users should be able to add plugins via the sipco
GUI, and then 
> sipco could take care of changing the felix.properties
file.

Indeed this is a bit risky since you'll have to modify this
file every 
time we modify the main felix properties file. I completely
agree with 
you that we should have this handled by a GUI plugin manager
... that we 
don't have yet.

Just an idea, until we get the plugin manager, wouldn't it
be easier for 
you to keep in the macosx felix properties file only the
line that 
starts the growlnotification bundle? In your mac specific
properties 
file you could have a single line that looks like this:

  felix.auto.start.70=
reference:file:sc-bundles/growlnotification.jar

and you could attach it to the end of the main 
felix.client.run.properties.

This should save you the effort of keeping a duplicate 
felix.client.run.properties up to date with the main one.

WDYT?

> - growlnotification.tar.gz: Implementation of the
service, which includes:
> 
> net/java/sip/communicator/impl/growlnotification/
> GrowlNotificationActivator.java
> GrowlNotificationServiceImpl.java
> growlnotification.manifest.mf
> 
> net/java/sip/communicator/service/growlnotification/
> GrowlNotificationService.java
> 
> The GrowlNotificationService.java is almost empty,
because I do not 
> define any new functions (I only implements
interfaces). So I wonder if 
> this one is useful.

Indeed not really. We might one day consider something like
a 
sysnotification service that other bundles could use to pop
up important 
messages to users but for now I am keeping the service part
outside.

> Now some other details:
> - I understood the principles of the slick, but I have
no idea how I can 
> implement a slick for this plugin. If you have any
hints...

Good question ... it would be quite delicate to add
consistent unit 
tests, not to mention running them on cruisecontrol since it
is 
installer on a linux machine and growl is not available on
it.

> About the TODOs:
> - Put the SIP communicator Icon as the default icon for
the 
> notifications (later, when SIP communicator will be
able to get the 
> people's avatar, we can display it instead in the
notification window)

agree

> - Implement other notifications, such as when a contact
becomes online 
> or offline, when a call is received, a file transfer
finished or aborted 
> etc.

yes but that would also require adding a growl pane in the
configuration 
window since users should have the possibility of specifying
what kind 
of events would they like to receive notifications for.

> - Do not display notifications for the contact whose
chat window is 
> active (and also if the user is on the same desktop as
the chat window).

This one should be quite easy actually. It amounts to
something like

  if( !
uiService.getChatWindow(srcContact).isWindowVisible() )
       return;

(Yana would correct me if I am wrong but it should be
something like
this).

Anyways, I am committing with only a few minor
modifications. Apart the 
points that I've addressed in my previous comments I've also
had to do 
on (uglier) change:

Calling notifier.notifyGrowlOf(...) requires cocoa in the
classpath 
during compilation (this means that even if we don't build
the bundle we 
still need cocoa). In order to avoid compilation errors on
non mac 
platforms, I've added a notifyGrowlOf() method in your 
GrowlNotificationServiceImpl class.

The method wraps usage of growl with reflection. This
resolves 
compilation issues but would only work until the day you
decide to add 
images to your notifications since at that point there would
be no way 
around and you'd have to have cocoa in your classpath.

We are planning to build a separate plugin repository on 
sip-communicator.dev.java.net and when this happens, the
growl notifier 
would have its own project and won't have to compile on
anything else 
but a mac ... until then reflection would have to do.

OK, I think that's all. Let me know if I've broken anything
(which is 
quite likely) or if you have any comments or disagree with
my 
modifications.

Once again, great work Romain!

Cheers
Emil

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hi Emil,

Emil Ivov wrote:
> Now isn't this just so cool 

Thanks Emil for applying this contribution to the
repository!
Actually it does not work from the CVS sources but here are
some patch 
to fix the issues. Some comments inline.

>> - growl.jar: a small library that needs to be put
in lib/
> 
> done. I only needed to recompile it so that it is 1.4
compatible. Please 
> try it and let me know if it doesn't work.

No problem to report with this new version.

>> - build.xml-20061207.patch: a patch to apply to
build.xml. It allows 
>> to not build the bundle if it is not compiled on
MacOSX, and to get a 
>> correct classpath for the "compile"
target if compilation is done on 
>> macosx (Be careful, this patch considers that the
previous patch I 
>> sent about sip-communicator.version has been
applied)
> 
> A couple of comments here. there's no point in
performing all the 
> classpath acrobatics. You can feel free to add
/System/Library/Java 
> directly to the project classpath since this won't
disturb non-mac 
> platforms (it would simply be ignored).

Yes indeed, this is a lot easier.

> I've also modified your bundling target to produce the
jar in 
> sc-bundles/os/macosx, in order to avoid having it
included in non-mac 
> distros that are copying * from sc-bundles. I've also
updated the macosx 
> target to get the growlnotification.jar file from
sc-bundles/os/macosx

Perfect 

>> - a felix.client.run.properties specific for
MacOSX: it includes the 
>> plugin and the growl lib. It must be put in
resources/install/macosx/
>> I think this could be a good temporary solution.
For the long term, I 
>> think users should be able to add plugins via the
sipco GUI, and then 
>> sipco could take care of changing the
felix.properties file.
> 
> Indeed this is a bit risky since you'll have to modify
this file every 
> time we modify the main felix properties file. I
completely agree with 
> you that we should have this handled by a GUI plugin
manager ... that we 
> don't have yet.
>
> Just an idea, until we get the plugin manager, wouldn't
it be easier for 
> you to keep in the macosx felix properties file only
the line that 
> starts the growlnotification bundle? In your mac
specific properties 
> file you could have a single line that looks like this:
> 
>  felix.auto.start.70=
reference:file:sc-bundles/growlnotification.jar

Yes sure, this is the best way. I enclosed two files:

- felix.client.run.properties.sed to put in
resources/install/macosx. 
Those are sed commands to add the specific macosx lines in
the 
felix.property file (the reference to the bundle, and the
com.growl in 
the osgi packages list)

- a patch to apply to the build.xml to execute a sed command
on the 
"official" felix.client.run.properties file, the
output being put in the 
resources/install/macosx directory.

Could you also delete the felix.client.run.properties file
that is in 
the resources/install/macosx on the repository ?

>> The GrowlNotificationService.java is almost empty,
because I do not 
>> define any new functions (I only implements
interfaces). So I wonder 
>> if this one is useful.
> 
> Indeed not really. We might one day consider something
like a 
> sysnotification service that other bundles could use to
pop up important 
> messages to users but for now I am keeping the service
part outside.

OK, I see that you did not include the file in the
repository. As you
say, there is no reason to do so at this time.

>> Now some other details:
>> - I understood the principles of the slick, but I
have no idea how I 
>> can implement a slick for this plugin. If you have
any hints...
> 
> Good question ... it would be quite delicate to add
consistent unit 
> tests, not to mention running them on cruisecontrol
since it is 
> installer on a linux machine and growl is not available
on it.

Ok, so as long as the code stays quite simple, I won't focus
on that.

>> - Implement other notifications, such as when a
contact becomes online 
>> or offline, when a call is received, a file
transfer finished or 
>> aborted etc.
> 
> yes but that would also require adding a growl pane in
the configuration 
> window since users should have the possibility of
specifying what kind 
> of events would they like to receive notifications for.

Actually, the great thing with Growl, is that you do not
need to
implement such control panel in your application because
Growls already 
comes with a control pane, where you can
activate/desactivate 
application or specific events that you registered to Growl.

Take a look again at the screenshot I sent in my last mail:
http:/
/www.rkuntz.org/sipco/sip-comm-growl.png

On the top right corner there is the Growl control pane
(located in the
MacOSX system preference), and you can configure all
application as you
like. In the screenshot, growl is configured to only report
events from
sip-communicator when it match the event "Message
received" or
"Sip-communicator Started".


>> - Do not display notifications for the contact
whose chat window is 
>> active (and also if the user is on the same desktop
as the chat window).
> 
> This one should be quite easy actually. It amounts to
something like
> 
>  if( !
uiService.getChatWindow(srcContact).isWindowVisible() )
>       return;
> 
> (Yana would correct me if I am wrong but it should be
something like
> this).

Actually I have another concern: message should still be
displayed if
the user is on another virtual desktop, even if the chat
window is 
active. So I have to find a way to get the desktop number of
the 
application, and the current used desktop number, and
according to that 
and the result of your test, the message would be displayed
or not.

I'll investigate that.

> Anyways, I am committing with only a few minor
modifications. Apart the 
> points that I've addressed in my previous comments I've
also had to do 
> on (uglier) change:
> 
> Calling notifier.notifyGrowlOf(...) requires cocoa in
the classpath 
> during compilation (this means that even if we don't
build the bundle we 
> still need cocoa). In order to avoid compilation errors
on non mac 
> platforms, I've added a notifyGrowlOf() method in your 
> GrowlNotificationServiceImpl class.
 >
> The method wraps usage of growl with reflection. This
resolves 
> compilation issues but would only work until the day
you decide to add 
> images to your notifications since at that point there
would be no way 
> around and you'd have to have cocoa in your classpath.
> 
> We are planning to build a separate plugin repository
on 
> sip-communicator.dev.java.net and when this happens,
the growl notifier 
> would have its own project and won't have to compile on
anything else 
> but a mac ... until then reflection would have to do.

Ok thanks for taking care of that. I must admit that I did
not try to 
compile it on non-mac platforms before submitting it.

There are just small issues preventing the bundle to work
correctly:

- There is a fix to apply to the
GrowlNotificationServiceImpl.java file 
(GrowlNotificationServiceImpl-fix.patch)

- The manifest file changed a bit from the one I submitted,
did you have 
any concern with it? Here is a patch to fix it in 
growlnotification.manifest.patch

Thanks!

-- 
Romain KUNTZ
kuntzsfc.wide.ad.jp

---
sip-communicator/src/net/java/sip/communicator/impl/growlnot
ification/GrowlNotificationServiceImpl.java	2006-12-14
07:46:08.000000000 +0900
+++
sip-communicator-fixed/src/net/java/sip/communicator/impl/gr
owlnotification/GrowlNotificationServiceImpl.java	2006-12-16
11:12:49.000000000 +0900
 -77,7
+77,7 
         try
         {
             Constructor constructor =
Growl.class.getConstructor(new Class[]
-                                       {String.class,
String.class, String.class});
+                                       {String.class,
String[].class, String[].class});
             notifier = (Growl)constructor.newInstance(
                     new Object[]{"SIP
Communicator", allNotif, defaultNotif});
             notifier.register();
---
sip-communicator/src/net/java/sip/communicator/impl/growlnot
ification/growlnotification.manifest.mf	2006-12-14
07:46:16.000000000 +0900
+++
sip-communicator-fixed/src/net/java/sip/communicator/impl/gr
owlnotification/growlnotification.manifest.mf	2006-12-16
11:49:28.000000000 +0900
 -7,3
+7,4 
  net.java.sip.communicator.util,
  net.java.sip.communicator.service.protocol,
  net.java.sip.communicator.service.protocol.event
+Export-Package: com.growl
s/org.osgi.framework.system.packages=/org.osgi.framework.sys
tem.packages= com.growl;/
$a
felix.auto.start.70=
reference:file:sc-bundles/os/macosx/growlnotification.jar
--- sip-communicator/build.xml	2006-12-16 02:00:34.000000000
+0900
+++ sip-communicator-fixed/build.xml	2006-12-16
14:32:25.000000000 +0900
 -331,6
+331,15 
                 trim="true"/>
       </condition>
 
+      <!-- Prepare the felix.client.run.properties file
for macosx -->
+      <exec executable="/usr/bin/sed" 
+          dir="$"
+         
input="$/felix.client.run.properties"
+         
output="${macosx.resrc.dir}/felix.client.run.properties
">
+        <arg value="-f"/>
+        <arg
value="${macosx.resrc.dir}/felix.client.run.properties.
sed"/>
+      </exec>
+
       <!-- Delete the old .app if it exists -->
       <delete
dir="${macosx.app.dir}/${macosx.app.name}-${sip-communi
cator.version}.app"
               quiet="yes"
failonerror="false"/>
 -376,7
+385,7 
           <exclude
name="${bundles.dest}/*-slick.jar" />
         </jarfileset>
         <javafilelist dir="$"
files="logging.properties"/>
-        <javafilelist
dir="resources/install/macosx"
+        <javafilelist
dir="${macosx.resrc.dir}"
                      
files="felix.client.run.properties"/>
 
       </jarbundler>

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hey Romain,

Romain KUNTZ wrote:
>>  felix.auto.start.70=
reference:file:sc-bundles/growlnotification.jar
> 
> Yes sure, this is the best way. I enclosed two files:
> 
> - felix.client.run.properties.sed to put in
resources/install/macosx. 
> Those are sed commands to add the specific macosx lines
in the 
> felix.property file (the reference to the bundle, and
the com.growl in 
> the osgi packages list)

done.

> Could you also delete the felix.client.run.properties
file that is in 
> the resources/install/macosx on the repository ?

done

> Actually I have another concern: message should still
be displayed if
> the user is on another virtual desktop, even if the
chat window is 
> active. So I have to find a way to get the desktop
number of the 
> application, and the current used desktop number, and
according to that 
> and the result of your test, the message would be
displayed or not.

Ah true. I didn't think of that. I am afraid that the we'd
also have 
this problem on Linux too and resolving it is going to be
tricky. The 
thing is that I am not sure whether the java application
itself realizes 
that it is on a (non-)visible desktop.

You are right we need to think more on that.

> There are just small issues preventing the bundle to
work correctly:
> 
> - There is a fix to apply to the
GrowlNotificationServiceImpl.java file 
> (GrowlNotificationServiceImpl-fix.patch)

done

> 
> - The manifest file changed a bit from the one I
submitted, did you have 
> any concern with it? Here is a patch to fix it in 
> growlnotification.manifest.patch

Oh yes. I see I messed that one up and forgot to add
com.growl to the 
import list.

Actually, I removed the exports clause because a bundle has
to export 
only interfaces and/or classes that it contains and that
other bundles 
would need to import. Most often these are the service APIs.
Right now 
com.growl is one of the system packages because your sed
script adds it 
to felix.client.run.properties and as a system package it is
not 
included in growlnotification.jar. Therefore it is not 
growlnotification.jar's responsibility to export it. Other
bundles that 
need this package would be able to import it directly from
the system 
packages.

I am adding com.growl to the Import-Package list of 
growlnotification.jar instead. This should be enough.
Please, let me 
know if it's working now or if I've messed it up again .

Cheers
Emil

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hi Emil,

Emil Ivov wrote:
> Romain KUNTZ wrote:
>>>  felix.auto.start.70=
reference:file:sc-bundles/growlnotification.jar
>>
>> Yes sure, this is the best way. I enclosed two
files:
>>
>> - felix.client.run.properties.sed to put in
resources/install/macosx. 
>> Those are sed commands to add the specific macosx
lines in the 
>> felix.property file (the reference to the bundle,
and the com.growl in 
>> the osgi packages list)
> 
> done.
> 
>> Could you also delete the
felix.client.run.properties file that is in 
>> the resources/install/macosx on the repository ?
> 
> done

I think you forgot to apply a patch to build.xml 
(build.xml-macosx-sed.patch, I enclosed it again), that
allows to apply 
the sed rules to the felix.properties file in order to
create a new 
specific macosx file.

>> - The manifest file changed a bit from the one I
submitted, did you 
>> have any concern with it? Here is a patch to fix it
in 
>> growlnotification.manifest.patch
> 
> Oh yes. I see I messed that one up and forgot to add
com.growl to the 
> import list.
> 
> Actually, I removed the exports clause because a bundle
has to export 
> only interfaces and/or classes that it contains and
that other bundles 
> would need to import. Most often these are the service
APIs. Right now 
> com.growl is one of the system packages because your
sed script adds it 
> to felix.client.run.properties and as a system package
it is not 
> included in growlnotification.jar. Therefore it is not 
> growlnotification.jar's responsibility to export it.
Other bundles that 
> need this package would be able to import it directly
from the system 
> packages.
> 
> I am adding com.growl to the Import-Package list of 
> growlnotification.jar instead. This should be enough.
Please, let me 
> know if it's working now or if I've messed it up again
.

Ok, beside the problem with the build.xml, it works fine !

Thanks,

-- 
Romain KUNTZ
kuntzsfc.wide.ad.jp
--- sip-communicator/build.xml	2006-12-16 02:00:34.000000000
+0900
+++ sip-communicator-fixed/build.xml	2006-12-16
14:32:25.000000000 +0900
 -331,6
+331,15 
                 trim="true"/>
       </condition>
 
+      <!-- Prepare the felix.client.run.properties file
for macosx -->
+      <exec executable="/usr/bin/sed" 
+          dir="$"
+         
input="$/felix.client.run.properties"
+         
output="${macosx.resrc.dir}/felix.client.run.properties
">
+        <arg value="-f"/>
+        <arg
value="${macosx.resrc.dir}/felix.client.run.properties.
sed"/>
+      </exec>
+
       <!-- Delete the old .app if it exists -->
       <delete
dir="${macosx.app.dir}/${macosx.app.name}-${sip-communi
cator.version}.app"
               quiet="yes"
failonerror="false"/>
 -376,7
+385,7 
           <exclude
name="${bundles.dest}/*-slick.jar" />
         </jarfileset>
         <javafilelist dir="$"
files="logging.properties"/>
-        <javafilelist
dir="resources/install/macosx"
+        <javafilelist
dir="${macosx.resrc.dir}"
                      
files="felix.client.run.properties"/>
 
       </jarbundler>


------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hey,

Romain KUNTZ wrote:
> I think you forgot to apply a patch to build.xml 
> (build.xml-macosx-sed.patch, I enclosed it again), that
allows to apply 
> the sed rules to the felix.properties file in order to
create a new 
> specific macosx file.

You are right. Silly me! It should be ok now.

> Ok, beside the problem with the build.xml, it works
fine !

Excellent, so now we have a growl notifier!

Thank you for you contribution Romain!

Cheers
Emil

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net