Summary: | ASTERISK-17866: [patch] MWI last-msgs-sent is mis-reported | ||||
Reporter: | Steve Davies (one47) | Labels: | |||
Date Opened: | 2011-05-16 08:15:04 | Date Closed: | 2012-05-23 08:13:52 | ||
Priority: | Minor | Regression? | No | ||
Status: | Closed/Complete | Components: | Channels/chan_sip/Subscriptions | ||
Versions: | 1.6.2.18 1.8-digiumphones-beta1 | Frequency of Occurrence | |||
Related Issues: |
| ||||
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. |