[Home]

Summary:ASTERISK-17866: [patch] MWI last-msgs-sent is mis-reported
Reporter:Steve Davies (one47)Labels:
Date Opened:2011-05-16 08:15:04Date Closed:2012-05-23 08:13:52
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Subscriptions
Versions:1.6.2.18 1.8-digiumphones-beta1 Frequency of
Occurrence
Related
Issues:
is related toASTERISK-18002 Voicemail MWI no longer is sent
Environment:Attachments:( 0) ast-17866-rb1272.patch
( 1) sip_mwi_fix-1.8.patch
( 2) sip_mwi.patch
Description:Found on 1.6.2.18, but as far as I can tell, this is still true in 1.8/trunk.

pabx*CLI> sip show peer peername
[snip]
 Mailbox      : 204
 VM Extension : asterisk_204
 LastMsgsSent : 32767/65535

LastMsgsSent is always incorrect.

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

Looking at the source code, the above output uses p->lastmsgssent variable. This variable is only ever set to -1, and is never set to its correct value. This is a hang-over from the way chan_sip used to send MWI state in Asterisk 1.2, it appears that a line of code was removed in error during the migration to event-driven MWI.

At the same time I notice that handle_request_register() no-longer causes a MWI update to be sent, because it is relying on the setting of p->lastmsgssent, which no-longer applies with event-driven voicemail notifications.
Comments:By: Steve Davies (one47) 2011-05-16 08:18:59

Attaching a possible patch, which compiles, but which I have not yet tested. It raises 2 questions:

1) Do I need additional locking around sip_send_mwi_to_peer() or is the ao2_ref that we hold sufficient? I believe handle_request_* functions are all called with the peer locked?

2) Should sip_send_mwi_to_peer() exit early if p->lastmsgssent is unchanged? The asterisk 1.2 code needed to do this, but it is "optional" now that we receive events for changes to MWI, and the test was removed at some point.



EDIT: Compiles and passes initial basic tests.

Rebooting a snom handset (which registers but does not subscribe MWI) now gets MWI update after REGISTER.

"sip show peer peername" now shows correct LastMsgsSent value.



By: Leif Madsen (lmadsen) 2011-05-17 07:27:09

This feels like a candidate for reviewboard. Thanks for the patch!

By: Steve Davies (one47) 2011-05-17 07:29:31

I've never dealt with the reviewboard system - Is there an FAQ/HOWTO you can direct me to?

Thanks.

By: Satish Patel (satish_lx) 2011-06-09 09:58:44.096-0500

AH! look like we have same issue. My asterisk 1.8.x not sending NOTIFY message to my polycom 501 but it does sent to X-lite softphone.

Any solution for 1.8?

By: Steve Davies (one47) 2011-06-09 10:09:51.475-0500

As far as I can see, the patch is identical for 1.6.2.18 and 1.8 trunk, bar a couple of thousand lines of offset, so try applying the patch above. If not, let me know and I will post an updated patch specifically for 1.8.

I've now load-tested this change with no obvious issues.


By: Gregory Hinton Nietsky (irroot) 2011-06-14 11:03:48.741-0500


Ok there is a gottcha here im running into here the events are not handled at all
event.c ast_event_check_subscriber returns AST_EVENT_SUB_NONE for all AST_EVENT_MWI with or
with out subscribers....



By: Gregory Hinton Nietsky (irroot) 2011-06-15 01:27:50.248-0500

Hi there please look at this with ASTERISK-18002 the extra hunk should allow subscriptions to events to work properly again.

By: Gregory Hinton Nietsky (irroot) 2011-06-15 02:26:50.918-0500

Realtime peers did not work as expected.

added a additional case for realtime peer registration and un registration.

By: Leif Madsen (lmadsen) 2011-06-15 07:35:10.680-0500

OK tested the patch without realtime -- working as expected! I was able to reproduce prior to the patch, and with the patch, the problem is resolved.

By: Gregory Hinton Nietsky (irroot) 2011-06-15 08:13:07.343-0500

there is massive confusion regarding the operation of the sip option subscribemwi should mwi be sent only when subscribed or if subscribed.

By: Steve Davies (one47) 2011-06-15 09:02:49.879-0500

With subscribemwi=no (the default): Registering a SIP peer creates a sort of implied MWI subscription. Many SIP handsets assume this is true and have no MWI subscribe facility.

With subscribemwi=yes             : Registering a SIP peer does not cause MWI notifies. The handset must separately subscribe if it wants MWI notifies.

The change in sip_mwi_irroot2.patch looks correct to me, but I do not use realtime, so have not tested it directly.

From sip.conf.sample:
;subscribemwi=yes       ; Only send notifications if this phone
                       ; subscribes for mailbox notification



By: Gregory Hinton Nietsky (irroot) 2011-06-15 10:01:42.921-0500

Thank you the comments in sip.h and chan_sip.c around this subscribemwi option
with the comments in sip.conf are confusing. and i need to make it clear in my head.

ill look at it with fresh eyes.

By: Gregory Hinton Nietsky (irroot) 2011-06-15 13:22:36.761-0500

Taking the above into account rework the patch to respect subscribemwi.

By: David Brillert (aragon) 2011-06-23 13:39:12.846-0500

I just tested Gregory Hinton Nietsky latest patch.
It fixes one problem but does not deal with another.
I have a 6010 extension (Polycom IP550) configured with mailbox and MWI works as it should with patch.
But extension 6003 (Polycom IP450) monitors mailbox 6010 and never sees the MWI light.

[6003]
type            =  friend
transport       =  udp
mohinterpret    =  g722
mohsuggest      =  g722
subscribecontext =  default-local
accountcode     =  6003
amaflags        =  default
parkinglot      =  parkinglot_default
defaultuser     =  6003
secret          =  secret
host            =  dynamic
language        =  en
callgroup       =  1
pickupgroup     =  1
t38pt_udptl     =  no
dtmfmode        =  rfc2833
callerid        =  "6003" <6003>
qualify         =  2000
trustrpid       =  no
sendrpid        =  yes
nat             =  no
canreinvite     =  no
mailbox         =  6003@default, 6010@default
disallow        =  all
allow           =  g722
allow           =  ulaw
context         =  default-super
cc_agent_policy =  generic
cc_monitor_policy =  generic
cc_offer_timer  =  20
setvar          =  EXTCONTEXT=default-super
setvar          =  TRANSFER_CONTEXT=default-super
setvar          =  FORCE_RECORDING=6003
setvar          =  AUTO_RECORDING=6003
setvar          =  INBOUND_GROUP=6003@INCOMING
setvar          =  SPYGROUP=default
call-limit      =  4
limitonpeers    =  yes
notifyringing   =  yes
notifyhold      =  yes
subscribemwi    =  yes

asterisk -rx 'sip show peer 6010' | grep LastMsgsSent
 LastMsgsSent : 1/0

asterisk -rx 'sip show peer 6003' | grep LastMsgsSent
 LastMsgsSent : 32767/65535



By: David Brillert (aragon) 2011-06-23 13:40:41.122-0500

same results if subscribemwi = no

By: Gregory Hinton Nietsky (irroot) 2011-06-23 14:18:57.488-0500

https://reviewboard.asterisk.org/r/1272/

starting with the first patch and ignoring the additional bits there has been some changes made and additional requirements.

the pro problem with peer expiring from cache and been reloaded with out lastmsgsent still exists need to make this persist in db/rt and will be for trunk dont think it should be for < 1.10

it adds some requested checks.

By: David Brillert (aragon) 2011-06-23 15:09:28.145-0500

Thanks for the patch Gregory.
Now both mailboxes MWI working when I leave a message in mailbox 6010
6003 monitors 6010
But MWI does not clear from 6003 afer 6010 deletes all messages.

and...
asterisk -rx 'sip show peer 6003' | grep LastMsgsSent
 LastMsgsSent : 32767/65535

asterisk -rx 'sip show peer 6010' | grep LastMsgsSent
 LastMsgsSent : 3/0



By: Gregory Hinton Nietsky (irroot) 2011-06-26 05:41:31.394-0500

@aargon please see ASTERISK-18002 the fix has been commited into svn branches/1.8

if you runing <= 1.8.4.x it will not be in there the events will not be sent to the phones the MWI events would not ever reach the phone.

By: David Brillert (aragon) 2011-06-26 10:58:09.252-0500

Hi Gregory results posted above are from 1.8 svn co and manually adding your patch.
Your patch adds the MWI to both mailboxes but nothing clears MWI after deleting the messages except restarting Asterisk...

To reproduce:
mailbox = 6003@default, 6010@default

By: Richard Mudgett (rmudgett) 2011-06-27 09:17:41.064-0500

You might want to look at ASTERISK-17997.  It might clear up the last issue you are having.

By: Gregory Hinton Nietsky (irroot) 2011-06-27 09:25:47.279-0500

@richard want to look at RB1272 again ?? perhaps ship this with 1.8.5 ??

By: Gregory Hinton Nietsky (irroot) 2011-06-27 09:25:50.504-0500

@richard want to look at RB1272 again ?? perhaps ship this with 1.8.5 ??

By: David Brillert (aragon) 2011-06-27 10:16:25.967-0500

rmudgett: I tested the patch at #17997 and I have reported my findings there.

By: David Brillert (aragon) 2011-06-27 11:24:58.803-0500

​Created new jira report at ASTERISK-18067 to document my issue.  I am no longer seeing the LastMsgsSent : 32767/65535 since patching with RB1272 and patch from 17997 on 1.8 SVN

By: Matt Hoskins (matthoskins2617) 2011-10-12 13:31:32.488-0500

Has this issue been resolved in > 1.8.6?  I have this problem with lastmsgssent being 32767/65535 and no MWI NOTIFY after initial register.


By: Richard Mudgett (rmudgett) 2011-10-12 13:48:58.210-0500

It does not look like this patch was committed to v1.8.

By: Gregory Hinton Nietsky (irroot) 2011-10-13 03:14:32.503-0500

1)Set mwi lastmsgssent correctly
2)refactor code to return consistant value in sip_send_mwi_to_peer
3)revert undocumented change of not sending mwi notify on register by default when registered

By: Scott Cechovic (sacechovic) 2012-04-10 13:02:50.601-0500

SVN-branch-1.8-digiumphones-r358725 still seeing issue.  


srv-4m*CLI> sip show peer 8003


 * Name       : 8003
 Realtime peer: Yes, cached
 Secret       : <Set>
 MD5Secret    : <Not set>
 Remote Secret: <Not set>
 Context      : localPHONES
 Subscr.Cont. : localPHONES
 Language     :
 Accountcode  : 100
 AMA flags    : Unknown
 Transfer mode: open
 CallingPres  : Presentation Allowed, Not Screened
 Callgroup    :
 Pickupgroup  :
 MOH Suggest  :
 Mailbox      : 8003@default
 VM Extension : asterisk
 LastMsgsSent : 32767/65535


By: Steve Davies (one47) 2012-04-11 03:36:27.146-0500

Added 1.8-digiumphones-beta1 to affected versions field as per report above.