[sip-comm-dev] Keybinding Chooser Addition


#1

Hi. I've just finished integrating a chooser for keybindings into the SIP
Communicator. This both provides a means for users to change their
keybinding preferences and save them between runs. Most of the details of
the addition are described in the latest blog post
here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
two additions:
1. Keybinding persistence via a new service. This saves custom bindings with
the user's preferences and informs any frames using the bindings of updates.
*src/net/java/sip/communicator/impl/keybindings
src/net/java/sip/communicator/service/keybindings

sipKeybindings.patch (70.9 KB)

KeybindingUtil.jar (160 KB)

···

*
2. Plugin that provides a ConfigurationForm for changing the keybindings.
*src/net/java/sip/communicator/plugin/keybindingChooser*

I've added a how-to for adding new keybinding sets
here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
chooser description). Also, this is my first time making a patch
- are binaries supposed to be provided separately like this? I didn't spot
an option to include them in svn diff...

A couple of usability features (any opinions?):
1. Add a key to revert bindings to defaults. Helpful or unnecessary?
2. Keybindings aren't changed until the user hits "Apply". Perhaps bindings
that haven't been applied should be discolored to draw attention to them?

Known Issue:
Not all permissible bindings work in all situations. For instance in the
main frame the 'Contacts' and 'Chat rooms' panels appear to intercept some
keyboard events, preventing bindings to directional arrows (without
modifiers) or the space bar from working. This isn't a terribly big woop but
might possibly confuse users.

A few unrelated things I've come across:
1. The patch also includes the fix for a minor (but very consistent)
spelling error: 'desactivate' to 'deactivate'. This included tweaking an
image with the word, changing some language files, and refactoring parts of
the ManageButtonsPanel class.

2. Does anyone know if there's a known issue with the chatalerter plugin?
Eclipse has complained that the org.jdesktop.jdic.misc package can't be
resolved ever since I checked it out via svn. However this has never
prevented the SIP Communicator from running. I've tried asking a couple of
times on the IRC channel but I think I keep catching everyone when they're
asleep...

3. Couple of minor issues on the site:
Developer Documentation > How to Configure Eclipse
Minor simplification- it's not necessary to get the old version of Subclipse
then upgrade it. The buckminster error can be avoided by disabling the
optional components.

Developer Documentation > UI Service > Create and add configuration forms
As Atul noticed the 'getConfigurationManager()' method doesn't exist and
should be 'getConfigurationWindow()'.

Hope this helps. Cheers! -Galen

PS. I'm planning to try going by my middle name, Galen, in SIP
correspondence to avoid confusion with Damien and the other Damian. :slight_smile:


#2

Hi Galen,

you've done a really great job in this patch!

First let me apologize for making you wait so long, we were really busy lately. We have discussed the plugin off list already, but it's now applied and committed. You're also on our contributors page.

(more inline)

Damian Johnson wrote:

Hi. I've just finished integrating a chooser for keybindings into the SIP
Communicator. This both provides a means for users to change their
keybinding preferences and save them between runs. Most of the details of
the addition are described in the latest blog post
here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
two additions:
1. Keybinding persistence via a new service. This saves custom bindings with
the user's preferences and informs any frames using the bindings of updates.
*src/net/java/sip/communicator/impl/keybindings
src/net/java/sip/communicator/service/keybindings

Applied. I like the way you have implemented key bindings in the gui bundle!

*
2. Plugin that provides a ConfigurationForm for changing the keybindings.
*src/net/java/sip/communicator/plugin/keybindingChooser*

I've added a how-to for adding new keybinding sets
here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
chooser description). Also, this is my first time making a patch
- are binaries supposed to be provided separately like this? I didn't spot
an option to include them in svn diff...

My reply is coming late, but yes binaries should be supplied separately, because they're not contained in the diff. The how-to is also very good idea. Let me know if you're interested in adding this on the SIP Communicator website.

A couple of usability features (any opinions?):
1. Add a key to revert bindings to defaults. Helpful or unnecessary?
2. Keybindings aren't changed until the user hits "Apply". Perhaps bindings
that haven't been applied should be discolored to draw attention to them?

My personal opinion is that a button "Defaults" could be really helpful. Otherwise coloring unsaved bindings could be useful, but also it could be confusing, the user could not be sure what exactly the color means.

Known Issue:
Not all permissible bindings work in all situations. For instance in the
main frame the 'Contacts' and 'Chat rooms' panels appear to intercept some
keyboard events, preventing bindings to directional arrows (without
modifiers) or the space bar from working. This isn't a terribly big woop but
might possibly confuse users.

Hm, could you explain more in details the problem situations, we could try to resolve this.

A few unrelated things I've come across:
1. The patch also includes the fix for a minor (but very consistent)
spelling error: 'desactivate' to 'deactivate'. This included tweaking an
image with the word, changing some language files, and refactoring parts of
the ManageButtonsPanel class.

Committed all spelling corrections. Thanks.

2. Does anyone know if there's a known issue with the chatalerter plugin?
Eclipse has complained that the org.jdesktop.jdic.misc package can't be
resolved ever since I checked it out via svn. However this has never
prevented the SIP Communicator from running. I've tried asking a couple of
times on the IRC channel but I think I keep catching everyone when they're
asleep...

Maybe you've already figured it out yourself, but I'm just replying for the record..Even that Eclipse is complaining about a library, if you compile and run SIP Communicator through Ant it will always work, because the classpath is set inside the build.xml. If Eclipse is complaining about a library this means that this library is not added in the .classpath file of Eclipse. You need to do the following: right click on the project / "Properties" / "Project Build Path" / "Libraries" / "Add new" / Find the library in the lib directory of SComm.

3. Couple of minor issues on the site:
Developer Documentation > How to Configure Eclipse
Minor simplification- it's not necessary to get the old version of Subclipse
then upgrade it. The buckminster error can be avoided by disabling the
optional components.

Nice catch! I didn't experienced this error, so I'm not very familiar with the problem, so again are you interested in changing the part of the tutorial concerning the problem.

Developer Documentation > UI Service > Create and add configuration forms
As Atul noticed the 'getConfigurationManager()' method doesn't exist and
should be 'getConfigurationWindow()'.

May be it's too late and I'm almost asleep :slight_smile: but I didn't get this one, could you be more precise?

Damian, the patch was very well documented, very mature and easy to read. Again you've done a very good work! Thank you for the patience and keep up the good work:)

Cheers,
Yana

···

Hope this helps. Cheers! -Galen

PS. I'm planning to try going by my middle name, Galen, in SIP
correspondence to avoid confusion with Damien and the other Damian. :slight_smile:

------------------------------------------------------------------------

------------------------------------------------------------------------

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hey Galen,

Nice work indeed and the feature is quite useful. I've just tried it and
I like it a lot. Thanks for contributing!

There are however a few things that make me feel uncomfortable with the
way this is currently integrated in SIP Communicator.

First, I don't quite see the need of splitting the implementation into a
service and a plugin. The configuration UI could very well live in
impl.keybinding. It would also be nice if we could copy necessary
classes from your lib into the service implementation. There are two
reasons I believe this would a good idea:
  - We'd probably need to make changes on some parts such as the
configuration UI and the persistence mechanisms (see below) so that they
would fit the SC conventions.
  - It would make the code easier to understand.

Then, storing default and user preferences for key bindings in a binary
form makes it difficult to change them for no real reason. I've just
seen that your lib already allows to store settings in a properties file
so I think this would fit better. We can put the default ones into the
defaults.properties files and then use the configuration service to
modify them.

What do you think?

Other than that I was wondering if I could make the following feature
requests:

1.Would it be possible to implement support for alternative actions?
Most key binding management UIs have it would be really nice if we also did.
and
2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
Ctrl-u shortcuts that we've recently added to the chat window?

Once again, thanks for contributing your work!

Emil

Yana Stamcheva написа:

···

Hi Galen,

you've done a really great job in this patch!

First let me apologize for making you wait so long, we were really busy
lately. We have discussed the plugin off list already, but it's now
applied and committed. You're also on our contributors page.

(more inline)

Damian Johnson wrote:

Hi. I've just finished integrating a chooser for keybindings into the SIP
Communicator. This both provides a means for users to change their
keybinding preferences and save them between runs. Most of the details of
the addition are described in the latest blog post
here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
two additions:
1. Keybinding persistence via a new service. This saves custom bindings with
the user's preferences and informs any frames using the bindings of updates.
*src/net/java/sip/communicator/impl/keybindings
src/net/java/sip/communicator/service/keybindings

Applied. I like the way you have implemented key bindings in the gui bundle!

*
2. Plugin that provides a ConfigurationForm for changing the keybindings.
*src/net/java/sip/communicator/plugin/keybindingChooser*

I've added a how-to for adding new keybinding sets
here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
chooser description). Also, this is my first time making a patch
- are binaries supposed to be provided separately like this? I didn't spot
an option to include them in svn diff...

My reply is coming late, but yes binaries should be supplied separately,
because they're not contained in the diff. The how-to is also very good
idea. Let me know if you're interested in adding this on the SIP
Communicator website.

A couple of usability features (any opinions?):
1. Add a key to revert bindings to defaults. Helpful or unnecessary?
2. Keybindings aren't changed until the user hits "Apply". Perhaps bindings
that haven't been applied should be discolored to draw attention to them?

My personal opinion is that a button "Defaults" could be really helpful.
Otherwise coloring unsaved bindings could be useful, but also it could
be confusing, the user could not be sure what exactly the color means.

Known Issue:
Not all permissible bindings work in all situations. For instance in the
main frame the 'Contacts' and 'Chat rooms' panels appear to intercept some
keyboard events, preventing bindings to directional arrows (without
modifiers) or the space bar from working. This isn't a terribly big woop but
might possibly confuse users.

Hm, could you explain more in details the problem situations, we could
try to resolve this.

A few unrelated things I've come across:
1. The patch also includes the fix for a minor (but very consistent)
spelling error: 'desactivate' to 'deactivate'. This included tweaking an
image with the word, changing some language files, and refactoring parts of
the ManageButtonsPanel class.

Committed all spelling corrections. Thanks.

2. Does anyone know if there's a known issue with the chatalerter plugin?
Eclipse has complained that the org.jdesktop.jdic.misc package can't be
resolved ever since I checked it out via svn. However this has never
prevented the SIP Communicator from running. I've tried asking a couple of
times on the IRC channel but I think I keep catching everyone when they're
asleep...

Maybe you've already figured it out yourself, but I'm just replying for
the record..Even that Eclipse is complaining about a library, if you
compile and run SIP Communicator through Ant it will always work,
because the classpath is set inside the build.xml. If Eclipse is
complaining about a library this means that this library is not added in
the .classpath file of Eclipse. You need to do the following: right
click on the project / "Properties" / "Project Build Path" / "Libraries"
/ "Add new" / Find the library in the lib directory of SComm.

3. Couple of minor issues on the site:
Developer Documentation > How to Configure Eclipse
Minor simplification- it's not necessary to get the old version of Subclipse
then upgrade it. The buckminster error can be avoided by disabling the
optional components.

Nice catch! I didn't experienced this error, so I'm not very familiar
with the problem, so again are you interested in changing the part of
the tutorial concerning the problem.

Developer Documentation > UI Service > Create and add configuration forms
As Atul noticed the 'getConfigurationManager()' method doesn't exist and
should be 'getConfigurationWindow()'.

May be it's too late and I'm almost asleep :slight_smile: but I didn't get this one,
could you be more precise?

Damian, the patch was very well documented, very mature and easy to
read. Again you've done a very good work! Thank you for the patience and
keep up the good work:)

Cheers,
Yana

Hope this helps. Cheers! -Galen

PS. I'm planning to try going by my middle name, Galen, in SIP
correspondence to avoid confusion with Damien and the other Damian. :slight_smile:

------------------------------------------------------------------------

------------------------------------------------------------------------

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hi guys, thanks for the replies!

The how-to is also very good idea. Let me know if you're interested in

adding this on the SIP Communicator website.

Certainly! Do I have access to edit the wiki? If not then feel free to snag
it.

My personal opinion is that a button "Defaults" could be really helpful.

Otherwise coloring unsaved bindings could be useful, but also it could be
confusing, the user could not be sure what exactly the color means.

Attached is a patch for a "use defaults" button and a couple small fixes. In
my opinion it would be easy for users to miss the 'apply' button and wonder
why their changes aren't working. The button's there so changes (which
include IO) are made in batches but perhaps I should do away with it in the
name of usability?

Hm, could you explain more in details the problem situations, we could try

to resolve this.

Here's the problematic use case:
1. Open Tools > Settings > Keybingings
2. In the 'Main' tab change the 'Next Tab' binding to the space bar (or
arrow keys)
3. Apply the change and close the settings window
4. Try to use the space bar to transverse tabs. It works if you're on the
'Call List' or 'Dial' tab but not if you're on 'Contacts' or 'Chat rooms'.

I'm not sure what it is about those panels that's intercepting certain
keystrokes. If it's default Swing behavior then this will probably be a
minor bug we'll encounter frequently.

May be it's too late and I'm almost asleep :slight_smile: but I didn't get this one,

could you be more precise?

The documentation uses a method that's been renamed. I'd be happy to fix the
documentation if I can. I'll probably feel foolish later for asking this but
where are the wiki's edit controls?

First, I don't quite see the need of splitting the implementation into a

service and a plugin.

Actually I was just thinking about this. Are we going to want plugins to be
able to listen for and process keyboard events? If so then the addition
shouldn't use the new methods provided via the SIPCommFrame. If not then
you're completely right.

The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making alternative
choosers (which I doubt) there's no need for it to act as a plugin.

It would also be nice if we could copy necessary classes from your lib into

the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting it to
conform to the coding guidelines would be a breeze. However, unless we're
gonna make radical changes to the UI I don't think this is a good idea.
Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor to
easily tweak the keybindings. If integrated we'd still probably need the jar
for that functionality (with the exception of the Persistence class that
package probably woulnd't be integrated).
2. As far as hacks go, what's used is pretty short and sweet (providing an
I18N label abstraction and stretching the layout a bit). I doubt breaking
open the jar would clarify anything- it would certainly give a lot more code
to look at.

Then, storing default and user preferences for key bindings in a binary form

makes it difficult to change them for no real reason. I've just seen that
your lib already allows to store settings in a properties file so I think
this would fit better.

I actually spent quite a while antagonizing over this point. The advantage
of storing it as a serialized linked hash map is that it retains ordering
information, which is important from a UI perspective (so options like 'next
tab' and 'previous tab' are shown next to each other). If that wasn't a
concern then certainly- viva la plain text.

We can put the default ones into the defaults.properties files and then use

the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look into
it.

1. Would it be possible to implement support for alternative actions? Most

key binding management UIs have it would be really nice if we also did.

I'm not familiar with this. Alternative action of what?

2. Would you be interested in adding support for the Ctrl-b, Ctrl-i, Ctrl-u

shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple once I
get to a beefier connection.

Thanks again for the feedback!

Cheers,
Galen

setDefault.diff (8.51 KB)

···

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Galen,

Nice work indeed and the feature is quite useful. I've just tried it and
I like it a lot. Thanks for contributing!

There are however a few things that make me feel uncomfortable with the
way this is currently integrated in SIP Communicator.

First, I don't quite see the need of splitting the implementation into a
service and a plugin. The configuration UI could very well live in
impl.keybinding. It would also be nice if we could copy necessary
classes from your lib into the service implementation. There are two
reasons I believe this would a good idea:
       - We'd probably need to make changes on some parts such as the
configuration UI and the persistence mechanisms (see below) so that they
would fit the SC conventions.
       - It would make the code easier to understand.

Then, storing default and user preferences for key bindings in a binary
form makes it difficult to change them for no real reason. I've just
seen that your lib already allows to store settings in a properties file
so I think this would fit better. We can put the default ones into the
defaults.properties files and then use the configuration service to
modify them.

What do you think?

Other than that I was wondering if I could make the following feature
requests:

1.Would it be possible to implement support for alternative actions?
Most key binding management UIs have it would be really nice if we also
did.
and
2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
Ctrl-u shortcuts that we've recently added to the chat window?

Once again, thanks for contributing your work!

Emil

Yana Stamcheva написа:
> Hi Galen,
>
> you've done a really great job in this patch!
>
> First let me apologize for making you wait so long, we were really busy
> lately. We have discussed the plugin off list already, but it's now
> applied and committed. You're also on our contributors page.
>
> (more inline)
>
> Damian Johnson wrote:
>> Hi. I've just finished integrating a chooser for keybindings into the
SIP
>> Communicator. This both provides a means for users to change their
>> keybinding preferences and save them between runs. Most of the details
of
>> the addition are described in the latest blog post
>> here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
>> two additions:
>> 1. Keybinding persistence via a new service. This saves custom bindings
with
>> the user's preferences and informs any frames using the bindings of
updates.
>> *src/net/java/sip/communicator/impl/keybindings
>> src/net/java/sip/communicator/service/keybindings
>
> Applied. I like the way you have implemented key bindings in the gui
bundle!
>
>> *
>> 2. Plugin that provides a ConfigurationForm for changing the
keybindings.
>> *src/net/java/sip/communicator/plugin/keybindingChooser*
>>
>> I've added a how-to for adding new keybinding sets
>> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
>> chooser description). Also, this is my first time making a patch
>> - are binaries supposed to be provided separately like this? I didn't
spot
>> an option to include them in svn diff...
>
> My reply is coming late, but yes binaries should be supplied separately,
> because they're not contained in the diff. The how-to is also very good
> idea. Let me know if you're interested in adding this on the SIP
> Communicator website.
>
>> A couple of usability features (any opinions?):
>> 1. Add a key to revert bindings to defaults. Helpful or unnecessary?
>> 2. Keybindings aren't changed until the user hits "Apply". Perhaps
bindings
>> that haven't been applied should be discolored to draw attention to
them?
>
> My personal opinion is that a button "Defaults" could be really helpful.
> Otherwise coloring unsaved bindings could be useful, but also it could
> be confusing, the user could not be sure what exactly the color means.
>
>> Known Issue:
>> Not all permissible bindings work in all situations. For instance in the
>> main frame the 'Contacts' and 'Chat rooms' panels appear to intercept
some
>> keyboard events, preventing bindings to directional arrows (without
>> modifiers) or the space bar from working. This isn't a terribly big woop
but
>> might possibly confuse users.
>
> Hm, could you explain more in details the problem situations, we could
> try to resolve this.
>
>> A few unrelated things I've come across:
>> 1. The patch also includes the fix for a minor (but very consistent)
>> spelling error: 'desactivate' to 'deactivate'. This included tweaking an
>> image with the word, changing some language files, and refactoring parts
of
>> the ManageButtonsPanel class.
>
> Committed all spelling corrections. Thanks.
>
>> 2. Does anyone know if there's a known issue with the chatalerter
plugin?
>> Eclipse has complained that the org.jdesktop.jdic.misc package can't be
>> resolved ever since I checked it out via svn. However this has never
>> prevented the SIP Communicator from running. I've tried asking a couple
of
>> times on the IRC channel but I think I keep catching everyone when
they're
>> asleep...
>
> Maybe you've already figured it out yourself, but I'm just replying for
> the record..Even that Eclipse is complaining about a library, if you
> compile and run SIP Communicator through Ant it will always work,
> because the classpath is set inside the build.xml. If Eclipse is
> complaining about a library this means that this library is not added in
> the .classpath file of Eclipse. You need to do the following: right
> click on the project / "Properties" / "Project Build Path" / "Libraries"
> / "Add new" / Find the library in the lib directory of SComm.
>
>> 3. Couple of minor issues on the site:
>> Developer Documentation > How to Configure Eclipse
>> Minor simplification- it's not necessary to get the old version of
Subclipse
>> then upgrade it. The buckminster error can be avoided by disabling the
>> optional components.
>
> Nice catch! I didn't experienced this error, so I'm not very familiar
> with the problem, so again are you interested in changing the part of
> the tutorial concerning the problem.
>
>> Developer Documentation > UI Service > Create and add configuration
forms
>> As Atul noticed the 'getConfigurationManager()' method doesn't exist and
>> should be 'getConfigurationWindow()'.
>
> May be it's too late and I'm almost asleep :slight_smile: but I didn't get this one,
> could you be more precise?
>
> Damian, the patch was very well documented, very mature and easy to
> read. Again you've done a very good work! Thank you for the patience and
> keep up the good work:)
>
> Cheers,
> Yana
>
>> Hope this helps. Cheers! -Galen
>>
>> PS. I'm planning to try going by my middle name, Galen, in SIP
>> correspondence to avoid confusion with Damien and the other Damian. :slight_smile:
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>> ------------------------------------------------------------------------
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
>> For additional commands, e-mail: dev-help@sip-communicator.dev.java.net
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
> For additional commands, e-mail: dev-help@sip-communicator.dev.java.net
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

Hey Damian,

Damian Johnson написа:

    First, I don't quite see the need of splitting the implementation
    into a service and a plugin.

Actually I was just thinking about this. Are we going to want plugins to
be able to listen for and process keyboard events?

Most probably.

If so then the
addition shouldn't use the new methods provided via the SIPCommFrame. If
not then you're completely right.

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should belong
to the same package.

    The configuration UI could very well live in impl.keybinding.

Certainly could. Assuming developers aren't interested in making
alternative choosers (which I doubt) there's no need for it to act as a
plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

    It would also be nice if we could copy necessary classes from your
    lib into the service implementation.

It would be easy to do- autoformatting is a beautiful thing so getting
it to conform to the coding guidelines would be a breeze. However,
unless we're gonna make radical changes to the UI I don't think this is
a good idea. Here's a couple of issues:

1. As the how-to mentions the jar is executable, providing an editor to
easily tweak the keybindings. If integrated we'd still probably need the
jar for that functionality (with the exception of the Persistence class
that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text form
then we won't need to use the standalone tool when generating or
modifying the default settings.

2. As far as hacks go, what's used is pretty short and sweet (providing
an I18N label abstraction and stretching the layout a bit). I doubt
breaking open the jar would clarify anything- it would certainly give a
lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

    Then, storing default and user preferences for key bindings in a
    binary form makes it difficult to change them for no real reason.
    I've just seen that your lib already allows to store settings in a
    properties file so I think this would fit better.

I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite often
and not being able to do so by editing a text file would be a problem.

The
advantage of storing it as a serialized linked hash map is that it
retains ordering information, which is important from a UI perspective
(so options like 'next tab' and 'previous tab' are shown next to each
other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by manually
adding indexes. Here's a very rough example:

....
<impl>
    <keybindings>
        <binding1>
            <actionName value="Previous Tab"/>
            ...
        </binding1>
        <binding2>
            <actionName value="Next Tab"/>
            ...
        </binding2>
    </keybindings>
</impl>

    We can put the default ones into the defaults.properties files and
    then use the configuration service to modify them.

It looks like this service has changed quite a bit recently. I'll look
into it.

Yes the defaults are new indeed.

    1. Would it be possible to implement support for alternative
    actions? Most key binding management UIs have it would be really
    nice if we also did.

I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

Oops! That's what I get for failing to sync with the trunk. Simple once
I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

Thanks again for the feedback!

Thank you Damian, I do like the feature very much!

Cheers
Emil

···

Cheers,
Galen

On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org > <mailto:emcho@sip-communicator.org>> wrote:

    Hey Galen,

    Nice work indeed and the feature is quite useful. I've just tried it and
    I like it a lot. Thanks for contributing!

    There are however a few things that make me feel uncomfortable with the
    way this is currently integrated in SIP Communicator.

    First, I don't quite see the need of splitting the implementation into a
    service and a plugin. The configuration UI could very well live in
    impl.keybinding. It would also be nice if we could copy necessary
    classes from your lib into the service implementation. There are two
    reasons I believe this would a good idea:
           - We'd probably need to make changes on some parts such as the
    configuration UI and the persistence mechanisms (see below) so that they
    would fit the SC conventions.
           - It would make the code easier to understand.

    Then, storing default and user preferences for key bindings in a binary
    form makes it difficult to change them for no real reason. I've just
    seen that your lib already allows to store settings in a properties file
    so I think this would fit better. We can put the default ones into the
    defaults.properties files and then use the configuration service to
    modify them.

    What do you think?

    Other than that I was wondering if I could make the following feature
    requests:

    1.Would it be possible to implement support for alternative actions?
    Most key binding management UIs have it would be really nice if we
    also did.
    and
    2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
    Ctrl-u shortcuts that we've recently added to the chat window?

    Once again, thanks for contributing your work!

    Emil

    Yana Stamcheva написа:
    > Hi Galen,
    >
    > you've done a really great job in this patch!
    >
    > First let me apologize for making you wait so long, we were really
    busy
    > lately. We have discussed the plugin off list already, but it's now
    > applied and committed. You're also on our contributors page.
    >
    > (more inline)
    >
    > Damian Johnson wrote:
    >> Hi. I've just finished integrating a chooser for keybindings into
    the SIP
    >> Communicator. This both provides a means for users to change their
    >> keybinding preferences and save them between runs. Most of the
    details of
    >> the addition are described in the latest blog post
    >> here<http://www.atagar.com/misc/gsocBlog/>but in short it consists of
    >> two additions:
    >> 1. Keybinding persistence via a new service. This saves custom
    bindings with
    >> the user's preferences and informs any frames using the bindings
    of updates.
    >> *src/net/java/sip/communicator/impl/keybindings
    >> src/net/java/sip/communicator/service/keybindings
    >
    > Applied. I like the way you have implemented key bindings in the
    gui bundle!
    >
    >> *
    >> 2. Plugin that provides a ConfigurationForm for changing the
    keybindings.
    >> *src/net/java/sip/communicator/plugin/keybindingChooser*
    >>
    >> I've added a how-to for adding new keybinding sets
    >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below the
    >> chooser description). Also, this is my first time making a patch
    >> - are binaries supposed to be provided separately like this? I
    didn't spot
    >> an option to include them in svn diff...
    >
    > My reply is coming late, but yes binaries should be supplied
    separately,
    > because they're not contained in the diff. The how-to is also very
    good
    > idea. Let me know if you're interested in adding this on the SIP
    > Communicator website.
    >
    >> A couple of usability features (any opinions?):
    >> 1. Add a key to revert bindings to defaults. Helpful or unnecessary?
    >> 2. Keybindings aren't changed until the user hits "Apply".
    Perhaps bindings
    >> that haven't been applied should be discolored to draw attention
    to them?
    >
    > My personal opinion is that a button "Defaults" could be really
    helpful.
    > Otherwise coloring unsaved bindings could be useful, but also it could
    > be confusing, the user could not be sure what exactly the color means.
    >
    >> Known Issue:
    >> Not all permissible bindings work in all situations. For instance
    in the
    >> main frame the 'Contacts' and 'Chat rooms' panels appear to
    intercept some
    >> keyboard events, preventing bindings to directional arrows (without
    >> modifiers) or the space bar from working. This isn't a terribly
    big woop but
    >> might possibly confuse users.
    >
    > Hm, could you explain more in details the problem situations, we could
    > try to resolve this.
    >
    >> A few unrelated things I've come across:
    >> 1. The patch also includes the fix for a minor (but very consistent)
    >> spelling error: 'desactivate' to 'deactivate'. This included
    tweaking an
    >> image with the word, changing some language files, and
    refactoring parts of
    >> the ManageButtonsPanel class.
    >
    > Committed all spelling corrections. Thanks.
    >
    >> 2. Does anyone know if there's a known issue with the chatalerter
    plugin?
    >> Eclipse has complained that the org.jdesktop.jdic.misc package
    can't be
    >> resolved ever since I checked it out via svn. However this has never
    >> prevented the SIP Communicator from running. I've tried asking a
    couple of
    >> times on the IRC channel but I think I keep catching everyone
    when they're
    >> asleep...
    >
    > Maybe you've already figured it out yourself, but I'm just
    replying for
    > the record..Even that Eclipse is complaining about a library, if you
    > compile and run SIP Communicator through Ant it will always work,
    > because the classpath is set inside the build.xml. If Eclipse is
    > complaining about a library this means that this library is not
    added in
    > the .classpath file of Eclipse. You need to do the following: right
    > click on the project / "Properties" / "Project Build Path" /
    "Libraries"
    > / "Add new" / Find the library in the lib directory of SComm.
    >
    >> 3. Couple of minor issues on the site:
    >> Developer Documentation > How to Configure Eclipse
    >> Minor simplification- it's not necessary to get the old version
    of Subclipse
    >> then upgrade it. The buckminster error can be avoided by
    disabling the
    >> optional components.
    >
    > Nice catch! I didn't experienced this error, so I'm not very familiar
    > with the problem, so again are you interested in changing the part of
    > the tutorial concerning the problem.
    >
    >> Developer Documentation > UI Service > Create and add
    configuration forms
    >> As Atul noticed the 'getConfigurationManager()' method doesn't
    exist and
    >> should be 'getConfigurationWindow()'.
    >
    > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
    this one,
    > could you be more precise?
    >
    > Damian, the patch was very well documented, very mature and easy to
    > read. Again you've done a very good work! Thank you for the
    patience and
    > keep up the good work:)
    >
    > Cheers,
    > Yana
    >
    >> Hope this helps. Cheers! -Galen
    >>
    >> PS. I'm planning to try going by my middle name, Galen, in SIP
    >> correspondence to avoid confusion with Damien and the other
    Damian. :slight_smile:
    >>
    >>
    >>
    >>
    ------------------------------------------------------------------------
    >>
    >>
    >>
    ------------------------------------------------------------------------
    >>
    >>
    >>
    ------------------------------------------------------------------------
    >>
    >> ---------------------------------------------------------------------
    >> To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
    >> For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>
    >
    > ---------------------------------------------------------------------
    > To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
    > For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>
    >
    >

    ---------------------------------------------------------------------
    To unsubscribe, e-mail:
    dev-unsubscribe@sip-communicator.dev.java.net
    <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
    For additional commands, e-mail:
    dev-help@sip-communicator.dev.java.net
    <mailto:dev-help@sip-communicator.dev.java.net>

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#7

Hi Emil,

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Ack! Sorry, I misread your original comment and thought the proposition was
to do away with the service. My bad.

I was trying to say that since the configuration UI is quite tightly
coupled with this service implementation and both are quite light then they
probably should belong to the same package.... It would have made sense to
keep them separated if there's a chance of someone needing concurrent
versions of the chooser UI, which I don't think is very probable.

Good points- I'll make the change.

Well unless I am missing something we'll probably only need what's in the
chooser package which gives us approximately 1400 lines of code. The main
reason I am insisting on this is that we'll want to modify the UI and having
the code in there would make it easier... If you don't feel like making the
change then, by all means, just leave it to us. Your contribution is
appreciated as it is

As I mentioned if we're gonna make substantial changes to the UI then this
makes sense, and the 'alternative bindings' are certainly substantial
enough. I'm sorry if I gave the impression that I wasn't willing to make the
change- I'll handle it.

... I meant alternative bindings

I've never seen this feature before but should be simple once the chooser's
source is added in.

Hopefully we'll soon be able to start integrating the spell checker soon
too. :slight_smile:

Cheers,
Damian

PS. I give up on going by 'Galen'. With webmail metadata adding in my name
this is just confusing. Oh well- it was worth a try...

···

On Fri, Jul 18, 2008 at 5:20 PM, Emil Ivov <emcho@sip-communicator.org> wrote:

Hey Damian,

Damian Johnson написа:
> First, I don't quite see the need of splitting the implementation
> into a service and a plugin.
>
> Actually I was just thinking about this. Are we going to want plugins to
> be able to listen for and process keyboard events?

Most probably.

> If so then the
> addition shouldn't use the new methods provided via the SIPCommFrame. If
> not then you're completely right.

I am not sure I see why this prevents us from copying the content of the
plugin.keybindingchooser package into impl.keybindings.

Guess I wasn't clear. Apologies. I was trying to say that since the
configuration UI is quite tightly coupled with this service
implementation and both are quite light then they probably should belong
to the same package.

> The configuration UI could very well live in impl.keybinding.
>
> Certainly could. Assuming developers aren't interested in making
> alternative choosers (which I doubt) there's no need for it to act as a
> plugin.

I agree, developers are likely to tackle the UI but since both the
plugin and the impl are quite light, merging them would not make
modifying the UI any more difficult. Quite, on the contrary, code would
be easier to read since it would all be in the same place.

It would have made sense to keep them separated if there's a chance of
someone needing concurrent versions of the chooser UI, which I don't
think is very probable.

> It would also be nice if we could copy necessary classes from your
> lib into the service implementation.
>
> It would be easy to do- autoformatting is a beautiful thing so getting
> it to conform to the coding guidelines would be a breeze. However,
> unless we're gonna make radical changes to the UI I don't think this is
> a good idea. Here's a couple of issues:
>
> 1. As the how-to mentions the jar is executable, providing an editor to
> easily tweak the keybindings. If integrated we'd still probably need the
> jar for that functionality (with the exception of the Persistence class
> that package probably woulnd't be integrated).

I was actually thinking that if we start saving everything in text form
then we won't need to use the standalone tool when generating or
modifying the default settings.

> 2. As far as hacks go, what's used is pretty short and sweet (providing
> an I18N label abstraction and stretching the layout a bit). I doubt
> breaking open the jar would clarify anything- it would certainly give a
> lot more code to look at.

Well unless I am missing something we'll probably only need what's in
the chooser package which gives us approximately 1400 lines of code. The
main reason I am insisting on this is that we'll want to modify the UI
and having the code in there would make it easier.

If you don't feel like making the change then, by all means, just leave
it to us. Your contribution is appreciated as it is.

> Then, storing default and user preferences for key bindings in a
> binary form makes it difficult to change them for no real reason.
> I've just seen that your lib already allows to store settings in a
> properties file so I think this would fit better.
>
> I actually spent quite a while antagonizing over this point.

The point is that we will probably need to modify defaults quite often
and not being able to do so by editing a text file would be a problem.

> The
> advantage of storing it as a serialized linked hash map is that it
> retains ordering information, which is important from a UI perspective
> (so options like 'next tab' and 'previous tab' are shown next to each
> other). If that wasn't a concern then certainly- viva la plain text.

You can easily implement this with our ConfigurationService by manually
adding indexes. Here's a very rough example:

....
<impl>
   <keybindings>
       <binding1>
           <actionName value="Previous Tab"/>
           ...
       </binding1>
       <binding2>
           <actionName value="Next Tab"/>
           ...
       </binding2>
   </keybindings>
</impl>

> We can put the default ones into the defaults.properties files and
> then use the configuration service to modify them.
>
> It looks like this service has changed quite a bit recently. I'll look
> into it.

Yes the defaults are new indeed.

> 1. Would it be possible to implement support for alternative
> actions? Most key binding management UIs have it would be really
> nice if we also did.
>
> I'm not familiar with this. Alternative action of what?

Sorry, my bad, I meant alternative bindings. (See attached PNG)

>
> 2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
> Ctrl-u shortcuts that we've recently added to the chat window?
>
> Oops! That's what I get for failing to sync with the trunk. Simple once
> I get to a beefier connection.

Thanks, I was hoping you'd say so :wink:

> Thanks again for the feedback!

Thank you Damian, I do like the feature very much!

Cheers
Emil
>
> Cheers,
> Galen
>
> On Fri, Jul 18, 2008 at 10:45 AM, Emil Ivov <emcho@sip-communicator.org > > <mailto:emcho@sip-communicator.org>> wrote:
>
> Hey Galen,
>
> Nice work indeed and the feature is quite useful. I've just tried it
and
> I like it a lot. Thanks for contributing!
>
> There are however a few things that make me feel uncomfortable with
the
> way this is currently integrated in SIP Communicator.
>
> First, I don't quite see the need of splitting the implementation
into a
> service and a plugin. The configuration UI could very well live in
> impl.keybinding. It would also be nice if we could copy necessary
> classes from your lib into the service implementation. There are two
> reasons I believe this would a good idea:
> - We'd probably need to make changes on some parts such as the
> configuration UI and the persistence mechanisms (see below) so that
they
> would fit the SC conventions.
> - It would make the code easier to understand.
>
> Then, storing default and user preferences for key bindings in a
binary
> form makes it difficult to change them for no real reason. I've just
> seen that your lib already allows to store settings in a properties
file
> so I think this would fit better. We can put the default ones into
the
> defaults.properties files and then use the configuration service to
> modify them.
>
> What do you think?
>
> Other than that I was wondering if I could make the following feature
> requests:
>
> 1.Would it be possible to implement support for alternative actions?
> Most key binding management UIs have it would be really nice if we
> also did.
> and
> 2. Would you be interested in adding support for the Ctrl-b, Ctrl-i,
> Ctrl-u shortcuts that we've recently added to the chat window?
>
> Once again, thanks for contributing your work!
>
> Emil
>
>
> Yana Stamcheva написа:
> > Hi Galen,
> >
> > you've done a really great job in this patch!
> >
> > First let me apologize for making you wait so long, we were really
> busy
> > lately. We have discussed the plugin off list already, but it's now
> > applied and committed. You're also on our contributors page.
> >
> > (more inline)
> >
> > Damian Johnson wrote:
> >> Hi. I've just finished integrating a chooser for keybindings into
> the SIP
> >> Communicator. This both provides a means for users to change their
> >> keybinding preferences and save them between runs. Most of the
> details of
> >> the addition are described in the latest blog post
> >> here<http://www.atagar.com/misc/gsocBlog/>but in short it
consists of
> >> two additions:
> >> 1. Keybinding persistence via a new service. This saves custom
> bindings with
> >> the user's preferences and informs any frames using the bindings
> of updates.
> >> *src/net/java/sip/communicator/impl/keybindings
> >> src/net/java/sip/communicator/service/keybindings
> >
> > Applied. I like the way you have implemented key bindings in the
> gui bundle!
> >
> >> *
> >> 2. Plugin that provides a ConfigurationForm for changing the
> keybindings.
> >> *src/net/java/sip/communicator/plugin/keybindingChooser*
> >>
> >> I've added a how-to for adding new keybinding sets
> >> here<http://www.atagar.com/misc/gsocBlog/keybindings.php>(below
the
> >> chooser description). Also, this is my first time making a patch
> >> - are binaries supposed to be provided separately like this? I
> didn't spot
> >> an option to include them in svn diff...
> >
> > My reply is coming late, but yes binaries should be supplied
> separately,
> > because they're not contained in the diff. The how-to is also very
> good
> > idea. Let me know if you're interested in adding this on the SIP
> > Communicator website.
> >
> >> A couple of usability features (any opinions?):
> >> 1. Add a key to revert bindings to defaults. Helpful or
unnecessary?
> >> 2. Keybindings aren't changed until the user hits "Apply".
> Perhaps bindings
> >> that haven't been applied should be discolored to draw attention
> to them?
> >
> > My personal opinion is that a button "Defaults" could be really
> helpful.
> > Otherwise coloring unsaved bindings could be useful, but also it
could
> > be confusing, the user could not be sure what exactly the color
means.
> >
> >> Known Issue:
> >> Not all permissible bindings work in all situations. For instance
> in the
> >> main frame the 'Contacts' and 'Chat rooms' panels appear to
> intercept some
> >> keyboard events, preventing bindings to directional arrows
(without
> >> modifiers) or the space bar from working. This isn't a terribly
> big woop but
> >> might possibly confuse users.
> >
> > Hm, could you explain more in details the problem situations, we
could
> > try to resolve this.
> >
> >> A few unrelated things I've come across:
> >> 1. The patch also includes the fix for a minor (but very
consistent)
> >> spelling error: 'desactivate' to 'deactivate'. This included
> tweaking an
> >> image with the word, changing some language files, and
> refactoring parts of
> >> the ManageButtonsPanel class.
> >
> > Committed all spelling corrections. Thanks.
> >
> >> 2. Does anyone know if there's a known issue with the chatalerter
> plugin?
> >> Eclipse has complained that the org.jdesktop.jdic.misc package
> can't be
> >> resolved ever since I checked it out via svn. However this has
never
> >> prevented the SIP Communicator from running. I've tried asking a
> couple of
> >> times on the IRC channel but I think I keep catching everyone
> when they're
> >> asleep...
> >
> > Maybe you've already figured it out yourself, but I'm just
> replying for
> > the record..Even that Eclipse is complaining about a library, if
you
> > compile and run SIP Communicator through Ant it will always work,
> > because the classpath is set inside the build.xml. If Eclipse is
> > complaining about a library this means that this library is not
> added in
> > the .classpath file of Eclipse. You need to do the following: right
> > click on the project / "Properties" / "Project Build Path" /
> "Libraries"
> > / "Add new" / Find the library in the lib directory of SComm.
> >
> >> 3. Couple of minor issues on the site:
> >> Developer Documentation > How to Configure Eclipse
> >> Minor simplification- it's not necessary to get the old version
> of Subclipse
> >> then upgrade it. The buckminster error can be avoided by
> disabling the
> >> optional components.
> >
> > Nice catch! I didn't experienced this error, so I'm not very
familiar
> > with the problem, so again are you interested in changing the part
of
> > the tutorial concerning the problem.
> >
> >> Developer Documentation > UI Service > Create and add
> configuration forms
> >> As Atul noticed the 'getConfigurationManager()' method doesn't
> exist and
> >> should be 'getConfigurationWindow()'.
> >
> > May be it's too late and I'm almost asleep :slight_smile: but I didn't get
> this one,
> > could you be more precise?
> >
> > Damian, the patch was very well documented, very mature and easy to
> > read. Again you've done a very good work! Thank you for the
> patience and
> > keep up the good work:)
> >
> > Cheers,
> > Yana
> >
> >> Hope this helps. Cheers! -Galen
> >>
> >> PS. I'm planning to try going by my middle name, Galen, in SIP
> >> correspondence to avoid confusion with Damien and the other
> Damian. :slight_smile:
> >>
> >>
> >>
> >>
>
------------------------------------------------------------------------
> >>
> >>
> >>
>
------------------------------------------------------------------------
> >>
> >>
> >>
>
------------------------------------------------------------------------
> >>
> >>
---------------------------------------------------------------------
> >> To unsubscribe, e-mail:
> dev-unsubscribe@sip-communicator.dev.java.net
> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
> >> For additional commands, e-mail:
> dev-help@sip-communicator.dev.java.net
> <mailto:dev-help@sip-communicator.dev.java.net>
> >
> >
---------------------------------------------------------------------
> > To unsubscribe, e-mail:
> dev-unsubscribe@sip-communicator.dev.java.net
> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
> > For additional commands, e-mail:
> dev-help@sip-communicator.dev.java.net
> <mailto:dev-help@sip-communicator.dev.java.net>
> >
> >
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> dev-unsubscribe@sip-communicator.dev.java.net
> <mailto:dev-unsubscribe@sip-communicator.dev.java.net>
> For additional commands, e-mail:
> dev-help@sip-communicator.dev.java.net
> <mailto:dev-help@sip-communicator.dev.java.net>
>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
> For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#8

Hello Thomas,

Sounds like the remote party is sending an SDP that makes us thing that
we are being put on hold. A wireshark dump of your session should helpd
debug the problem.

Cheers
Emil

Thomas Hofer написа:

···

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#9

I'll try my best to capture the right parts.
Cheers, thomas

···

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Montag, 21. Juli 2008 17:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: [sip-comm-dev] new Hold Button does not work

Hello Thomas,

Sounds like the remote party is sending an SDP that makes us thing that
we are being put on hold. A wireshark dump of your session should helpd
debug the problem.

Cheers
Emil

Thomas Hofer написа:

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#10

Just get the whole session and send it here so that we could look for
the problem. That should do.

Cheers
Emil

Thomas Hofer написа:

···

I'll try my best to capture the right parts.
Cheers, thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Montag, 21. Juli 2008 17:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: [sip-comm-dev] new Hold Button does not work

Hello Thomas,

Sounds like the remote party is sending an SDP that makes us thing that
we are being put on hold. A wireshark dump of your session should helpd
debug the problem.

Cheers
Emil

Thomas Hofer написа:

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#11

So. finally i think i got the point:
In CallSessionImpl.java in lin 1446 there is following code>

            if (!mediaServCallback.getDeviceConfiguration()
                .isVideoCaptureSupported())
            {
                /* We don't have anything to send. */
                onHold |= ON_HOLD_REMOTELY;
            }

This code puts the call on hold, if no video stream is available. Actually I do not think, this behavior is ok, because you cannot set a videostream as requirement. What do you think?

Cheers, Thomas

···

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Dienstag, 22. Juli 2008 09:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: AW: [sip-comm-dev] new Hold Button does not work

Just get the whole session and send it here so that we could look for
the problem. That should do.

Cheers
Emil

Thomas Hofer написа:

I'll try my best to capture the right parts.
Cheers, thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Montag, 21. Juli 2008 17:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: [sip-comm-dev] new Hold Button does not work

Hello Thomas,

Sounds like the remote party is sending an SDP that makes us thing that
we are being put on hold. A wireshark dump of your session should helpd
debug the problem.

Cheers
Emil

Thomas Hofer написа:

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#12

Actually i continued by debug work and the following code I do not understand:

Plese correct me, if I am wrong. But
-> calling with hold-status 0 (none set) shall not change anything.
-> calling with hold-status 0 (none set) is true on the first condition, so
--> onHold | ON_HOLD_LOCALLY = 2
--> onHold | ON_HOLD_LOCALLY = 4

So the method returns "inactive" when calling with 0 as value.
Is that the intended usage?

    private void setAttributeOnHold(MediaDescription mediaDescription,
        byte onHold) throws SdpException
    {
        String attribute;

        if (ON_HOLD_LOCALLY == (onHold | ON_HOLD_LOCALLY))
            attribute =
                (ON_HOLD_REMOTELY == (onHold | ON_HOLD_REMOTELY)) ? "inactive"
                        : "sendonly";
        else
            attribute =
                (ON_HOLD_REMOTELY == (onHold | ON_HOLD_REMOTELY)) ? "recvonly"
                        : null;

        if (attribute != null)
            mediaDescription.setAttribute(attribute, null);
    }

even

···

-----Ursprüngliche Nachricht-----
Von: Thomas Hofer [mailto:mailinglisten@familie-hofer.net]
Gesendet: Dienstag, 22. Juli 2008 09:39
An: dev@sip-communicator.dev.java.net
Betreff: AW: AW: [sip-comm-dev] new Hold Button does not work

So. finally i think i got the point:
In CallSessionImpl.java in lin 1446 there is following code>

            if (!mediaServCallback.getDeviceConfiguration()
                .isVideoCaptureSupported())
            {
                /* We don't have anything to send. */
                onHold |= ON_HOLD_REMOTELY;
            }

This code puts the call on hold, if no video stream is available. Actually I do not think, this behavior is ok, because you cannot set a videostream as requirement. What do you think?

Cheers, Thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Dienstag, 22. Juli 2008 09:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: AW: [sip-comm-dev] new Hold Button does not work

Just get the whole session and send it here so that we could look for
the problem. That should do.

Cheers
Emil

Thomas Hofer написа:

I'll try my best to capture the right parts.
Cheers, thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Montag, 21. Juli 2008 17:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: [sip-comm-dev] new Hold Button does not work

Hello Thomas,

Sounds like the remote party is sending an SDP that makes us thing that
we are being put on hold. A wireshark dump of your session should helpd
debug the problem.

Cheers
Emil

Thomas Hofer написа:

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#13

Thomas,

I'll be looking at it further but just wanted to share a quick opinion now:

The onHold in use in the code excerpt is a local variable which merges
the current onHold state (i.e. this.onHold) with the capabilities of
the respective media and is only used to determine the state of that
media (i.e. setAttributeOnHold for the MediaDescription in question).
In the absense of an on-hold condition (i.e. this.onHold doesn't have
any of the two on-hold flags raised), the code results in the
MediaDescription receiving an recvonly attribute which was the case
even before my modification.

Best regards,
Lubo

···

On Tue, Jul 22, 2008 at 10:39 AM, Thomas Hofer <mailinglisten@familie-hofer.net> wrote:

So. finally i think i got the point:
In CallSessionImpl.java in lin 1446 there is following code>

           if (!mediaServCallback.getDeviceConfiguration()
               .isVideoCaptureSupported())
           {
               /* We don't have anything to send. */
               onHold |= ON_HOLD_REMOTELY;
           }

This code puts the call on hold, if no video stream is available. Actually I do not think, this behavior is ok, because you cannot set a videostream as requirement. What do you think?

Cheers, Thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Dienstag, 22. Juli 2008 09:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: AW: [sip-comm-dev] new Hold Button does not work

Just get the whole session and send it here so that we could look for
the problem. That should do.

Cheers
Emil

Thomas Hofer написа:

I'll try my best to capture the right parts.
Cheers, thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Montag, 21. Juli 2008 17:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: [sip-comm-dev] new Hold Button does not work

Hello Thomas,

Sounds like the remote party is sending an SDP that makes us thing that
we are being put on hold. A wireshark dump of your session should helpd
debug the problem.

Cheers
Emil

Thomas Hofer написа:

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#14

In a summary of a private conversation with Thomas, the observation on
his side wrt the implementation of setAttributeOnHold is correct and
he has modified it to reflect the intention to check whether the
ON_HOLD_LOCALLY and/or ON_HOLD_REMOTELY flags are raised. As a result
of his fix, Thomas reported success in establishing calls.

Unfortunately, he experiences call failures on putting a held call off
hold. The reported fail point is the following code from
OperationSetBasicTelephonySipImpl#putOnHold(CallParticipant,boolean):

    sendInviteRequest(sipParticipant, callSession
                .createSdpDescriptionForHold(
                    sipParticipant.getSdpDescription(), on));

Regards,
Lubo

···

On Tue, Jul 22, 2008 at 11:07 AM, Thomas Hofer <mailinglisten@familie-hofer.net> wrote:

Actually i continued by debug work and the following code I do not understand:

Plese correct me, if I am wrong. But
-> calling with hold-status 0 (none set) shall not change anything.
-> calling with hold-status 0 (none set) is true on the first condition, so
--> onHold | ON_HOLD_LOCALLY = 2
--> onHold | ON_HOLD_LOCALLY = 4

So the method returns "inactive" when calling with 0 as value.
Is that the intended usage?

   private void setAttributeOnHold(MediaDescription mediaDescription,
       byte onHold) throws SdpException
   {
       String attribute;

       if (ON_HOLD_LOCALLY == (onHold | ON_HOLD_LOCALLY))
           attribute =
               (ON_HOLD_REMOTELY == (onHold | ON_HOLD_REMOTELY)) ? "inactive"
                       : "sendonly";
       else
           attribute =
               (ON_HOLD_REMOTELY == (onHold | ON_HOLD_REMOTELY)) ? "recvonly"
                       : null;

       if (attribute != null)
           mediaDescription.setAttribute(attribute, null);
   }

even

-----Ursprüngliche Nachricht-----
Von: Thomas Hofer [mailto:mailinglisten@familie-hofer.net]
Gesendet: Dienstag, 22. Juli 2008 09:39
An: dev@sip-communicator.dev.java.net
Betreff: AW: AW: [sip-comm-dev] new Hold Button does not work

So. finally i think i got the point:
In CallSessionImpl.java in lin 1446 there is following code>

           if (!mediaServCallback.getDeviceConfiguration()
               .isVideoCaptureSupported())
           {
               /* We don't have anything to send. */
               onHold |= ON_HOLD_REMOTELY;
           }

This code puts the call on hold, if no video stream is available. Actually I do not think, this behavior is ok, because you cannot set a videostream as requirement. What do you think?

Cheers, Thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Dienstag, 22. Juli 2008 09:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: AW: [sip-comm-dev] new Hold Button does not work

Just get the whole session and send it here so that we could look for
the problem. That should do.

Cheers
Emil

Thomas Hofer написа:

I'll try my best to capture the right parts.
Cheers, thomas

-----Ursprüngliche Nachricht-----
Von: Emil Ivov [mailto:emcho@sip-communicator.org]
Gesendet: Montag, 21. Juli 2008 17:17
An: dev@sip-communicator.dev.java.net
Betreff: Re: [sip-comm-dev] new Hold Button does not work

Hello Thomas,

Sounds like the remote party is sending an SDP that makes us thing that
we are being put on hold. A wireshark dump of your session should helpd
debug the problem.

Cheers
Emil

Thomas Hofer написа:

Hi!
I'm in big trouble using the new hold-button. On all calls the callstatus is initially set to "remotely hold". Additionally I cannot establish a call any more. Everthing keeps being silent.

Switching back to old versions (last Friday) of the following files solves this issue:
CallHistoryServiceImpl.java
CallPanel.java
CallParticipantPanel.java
HoldButton.java
OperationSetBasicTelephonySipImpl.java
CallSession.java
CallParticipantState.java
OperationSetBasicTelephony.java

Does anyone else have this problem?

Cheers
Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net