[Home]

Summary:ASTERISK-10824: Impliment CallForwardAll
Reporter:dea (dea)Labels:
Date Opened:2007-11-19 16:34:07.000-0600Date Closed:2008-01-14 12:33:02.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_skinny
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) cfwd.diff
( 1) chan_skinny-cfwall.txt
( 2) chan_skinny-cfwall-v2.txt
( 3) skinny_cfwd-11310.diff
( 4) skinny_cfwd-11310-2.diff
( 5) skinny_forward_message.txt
Description:The comment in chan_skinny saying that DND == CFwdAll has bothered
me for awhile now.

DND is supposed to simply mute the phones ringer and is a basic toggle.

CFwdAll should allow the user to enter a phone number to forward calls
to if not already set, or clear the current forwarding if it is set.



****** ADDITIONAL INFORMATION ******

I need to track down the softkey code for DND, as I do not want to
remove the feature, but do want it to reflect the correct button/action.

Call-Forward-Busy needs some more thought, and likely adding a call-limit
concept to chan_skinny.
Comments:By: dea (dea) 2007-11-19 16:36:29.000-0600

I should have mentioned that the attached patch impliments CFwdAll, and moves the code for DND.  A trivial patch is needed on top of it once the DND softkey
hex code has been identified.
By: Jason Parker (jparker) 2007-11-19 16:42:02.000-0600

I got a list of all the codes, at some point...  I wonder if I can find them.
By: dea (dea) 2007-11-19 19:33:19.000-0600

A list of 'event' codes was added to the chan_skinny source, but
the hex code listed for DND does not appear to match up with the
DND softkey.  If you find such a list, great.  I'll keep looking
in the mean time.
By: Jason Parker (jparker) 2007-11-20 10:18:39.000-0600

Ahh, I did add it.  Excellent.

You missed one very important detail while looking at those localization messages...they are in octal, not hex. :)

I believe 0x3F would be correct for SOFTKEY_DND.  Take a look at the definition of soft_key_template_default[] (you'll need to add DND there as well).  You can see there how there SOFTKEY_* definitions match up with the localization messages (hex vs octal).  \077 = 0x3F

Note that this is only true for the SOFTKEY definition, and not for the STIMULUS - I do not know what that one should be - if one even exists for it.

Besides - the softkeys mappings are pretty much arbitrary, since we handle it all at the server anyways.  The phone doesn't care what it sends, nor what it is labeled.
By: dea (dea) 2007-11-20 15:28:19.000-0600

V2 re-adds the DND button, and I also identified the iDivert button and
documented it in the defines, but there is no code to support it yet.

I also refactored the code to move the call forwarding logic to a
seperate function instead of duplicating it in the Stimulus and Softkey
handling functions.

The code is functional.  A couple of 'nice to have' additions would be
to play a short tone after collecting enough digits for a valid forward and
a similar tone when clearing the forwarding.  We also need to add update
the line state/device state to show that the phone is forwarded.  I'll try
to capture a CCM trace to identify what that message should look like.
By: Jason Parker (jparker) 2007-11-26 17:27:57.000-0600

Two things I see.

The reason SOFTKEY_DND has to be set to 0x13 is apparently due to a bug in the way softkeys are handled.  They have to be sequential..yuck.  When I wrote it that way, I knew it would eventually bite me.

When starting/stopping cfwdall, the phone gets an offhook event, then it gets told to play dialtone, and then onhook.  Why the brief dialtone?  Does CCM do that?
By: dea (dea) 2007-11-26 17:55:30.000-0600

I'm not sure what you mean by the 'sequential' comment.  I spent part of
an afternoon changing the onhook map to use raw hex values and recording
the buttons that showed up (edit, compile, install, start, repeat)

Yes, CCM does go offhook breifly when clearing the CFwdAll settings.  It
helps to convey to the user that they did press the button and something
did happen.  We can of course choose another way to communicate success.
By: Jason Parker (jparker) 2007-11-27 16:23:40.000-0600

How does this look? :)  It seems to be working pretty well here.

I added support for CFwdBusy also.  There are also hooks for CFwdNoAnswer, but it isn't implemented yet - I discussed it with file, and it's going to be pretty tricky.

You can enable BOTH busy and all, simultaneously, which I thought would be really useful.
By: dea (dea) 2007-11-27 17:05:35.000-0600

Looks good.  I stopped at CallForwardAll as it was the
one feature that could not be defined in the dialplan.

Busy and No Answer can have default behaviour defined in
the dialplan, just not easily overridden.  To handle the
no answer case, the channel would need to instruct app_dial
what to do when the call timeout (if configured) is reached.
There may well be value is add that concept to the core and
in app_dial.

I have not had time to finish with it, but I was able to
capture the packet structure and basic on/off values for the
skinny message that sets and clears the forwarded icon on
skinny phones.  I can upload a patch, or small text document
with the packet structure and on/off values, but I won't get
a chance to impliment it for awhile.
By: Jason Parker (jparker) 2007-11-27 17:13:55.000-0600

If you could post a doc on that, I can try to add it.  I assume it'll be relatively easy.
By: dea (dea) 2007-11-27 17:26:36.000-0600

Exceptionally easy I would think.  Paid work gets in the way, or
I'd have it done already.  The only issue, which may not be a
problem, is that I only was able to capture to message states
Forward enabled: 7
Forward disabled: 0

I suspect that there must be other states, otherwise 0 and 7 would
be odd choices.
By: Michiel van Baak (mvanbaak) 2008-01-11 15:09:02.000-0600

This one should make the indications works as well
By: Michiel van Baak (mvanbaak) 2008-01-11 15:58:17.000-0600

DEA:
I think the struct should be:
#define FORWARD_STAT_MESSAGE 0x0090
struct forward_stat_message {
       uint32_t activeforward;
       uint32_t lineNumber;
       uint32_t forwardall;
       char forwardallnum[24];
       uint32_t forwardBusy;
       char forwardBusynum[24];
       uint32_t forwardNoAns;
       char forwardNAnum[24];
};
By: dea (dea) 2008-01-11 16:38:26.000-0600

mvanbaak-
  I believe you may be right, at least it looks correct.
By: Michiel van Baak (mvanbaak) 2008-01-12 16:38:14.000-0600

I really dont understand the 7 in the status field.
I've tried 1-7 here and they all give the same result.

Cant we simply use 0 for off and 1 for on ?

btw, I'm talking about the uint32_t activeforward member of the forward_stat_message struct here.
By: Michiel van Baak (mvanbaak) 2008-01-13 11:56:37.000-0600

I tested cfwd.diff uploaded by Qwell today and it works fine on my 7960 phone here :)
By: dea (dea) 2008-01-13 12:07:13.000-0600

I am not sure on the 1 or 7 either, but from memory, when I wrote the
patch I was doing packet captures of the skinny events from CCM and those
are the only 2 values I found.
By: Digium Subversion (svnbot) 2008-01-14 11:38:25.000-0600

Repository: asterisk
Revision: 98776

U   trunk/channels/chan_skinny.c

------------------------------------------------------------------------
r98776 | qwell | 2008-01-14 11:38:23 -0600 (Mon, 14 Jan 2008) | 6 lines

Add proper call forwarding (all and busy) support for chan_skinny.
Note: NoAnswer support is currently not implemented, as it would take a
significant amount of work to figure out how to do correctly.

Closes issue ASTERISK-10824, patches, testing, and support by DEA, mvanbaak, and myself.

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

http://svn.digium.com/view/asterisk?view=rev&revision=98776
By: Jason Parker (jparker) 2008-01-14 12:33:02.000-0600

DEA, if you get some time, would you be willing/able to email me (qwell@digium.com) some packet traces of cfwdbusy?  The patch I committed would only give an icon with cfwdall.  I may have missed a critical piece, to get it working with cfwdbusy.