Summary:ASTERISK-15545: [patch] Add AMI support for device states
Reporter:Håkon Nessjøen (haakon)Labels:
Date Opened:2010-01-29 09:28:48.000-0600Date Closed:2015-03-14 17:01:12
Versions:Frequency of
Environment:Attachments:( 0) issue_16732-devicestate.patch
Description:main/devicestate.c says Asterisk Manager Interface (AMI) gets DeviceState events, but this is not true.

This patch adds a "DeviceStateChanged" event to AMI. And also introduces AMI commands DeviceStateSet and DeviceStateGet to controll device states from AMI, if func_devicestate is compiled in.
Comments:By: Håkon Nessjøen (haakon) 2010-01-29 09:43:10.000-0600

I forgot to add documentation for the manager commands.

Leif, is this good enough documentation? If so, I will add it, and upload a new patch at once. Else, give me pointers :)

       <manager name="DeviceStateGet" language="en_US">
                       Get DeviceState information.
                       <xi:include xpointer="xpointer(/docs/manager[@name='Login']/syntax/parameter[@name='ActionID'])" />
                       <parameter name="DeviceName" required="true" />
                       <para>The DeviceStateGet command can be used to get custom device states.</para>
                       <para>The result will be returned through a DeviceStateGetResponse event.</para>
       <manager name="DeviceStateSet" language="en_US">
                       Set DeviceState information.
                       <xi:include xpointer="xpointer(/docs/manager[@name='Login']/syntax/parameter[@name='ActionID'])" />
                       <parameter name="DeviceName" required="true" />
                       <parameter name="DeviceState" required="true" />
                       <para>The DeviceStateSet command can be used to set custom device states.</para>
                       <para>The possible DeviceState values for this function are:</para>
                       <para>UNKNOWN | NOT_INUSE | INUSE | BUSY | INVALID | UNAVAILABLE | RINGING |
                       RINGINUSE | ONHOLD</para>

By: Håkon Nessjøen (haakon) 2010-01-29 10:13:31.000-0600

Ok, I just saw that, if you have made a hint for a particular device state. Then you will get a ExtensionStatus event, if the state changes on that device.

This is probably good enough, so I can remove the new DeviceStateChanged from the patch.

But I still think the get and set commands are nice to have. I need it to update states about external mobile phones, that I get device information about from my mobile phone provider, for example.

By: Leif Madsen (lmadsen) 2010-01-29 12:46:19.000-0600

The documentation looks fine to me. If the DeviceStateChanged part is no longer necessary, then yes, please remove it. But if there is additional functionality here, then great! Thanks for the contribution! :)

By: Håkon Nessjøen (haakon) 2010-01-29 12:50:53.000-0600

I uploaded a new version, without the DeviceStateChanged part. The only difference with my "DeviceStateChanged" and the internal "ExtensionStatus" is that my version was always sent, whenever a device state was changed, but ExtensionStatus is sent as long as someone has configured a hint for that device. I guess you always configure hints for the channels you want to configure. So it's not that much of additional functionality in that.

New version also doesn't change CHANGES in the wrong section ;)

If you want, you can change the description to:
"This patch introduces AMI commands DeviceStateSet and DeviceStateGet to controll device states from AMI, as long as func_devicestate is selected for compiling."

By: Leif Madsen (lmadsen) 2010-01-29 15:16:46.000-0600

Actually, after talking to Russell, he thinks that the DeviceStateChanged might be useful still, because the ExtensionState may actually refer to multiple devices.

By: Håkon Nessjøen (haakon) 2010-01-29 15:23:42.000-0600

Added a new file with DeviceStateChanged added back again :)

By: Olle Johansson (oej) 2010-02-01 01:43:18.000-0600

Good patch, thanks!

When there is only one possible response, you should send the response in the response, not in an additional event.

Manager always subscribes to all state changes - is it really needed to have one more? If so, it has to be configurable since we will then have three events for every state change in SIP devices - on from the chan_sip file, one from manager extension state and one for the device state change. I'm not sure it should be in the CALL category either.

By: Olle Johansson (oej) 2010-02-01 01:45:13.000-0600

Read Leif's comment about Russell's comment again, so yes, if you want that level of details Device state notifications make sense, but I still think it has to be configurable. For a large installation, it will generate quite a lot of events. If we move it to a specific category, people can turn it on/off in manager.conf.

By: Håkon Nessjøen (haakon) 2010-02-01 02:36:13.000-0600

I agree that it might be a little much events if you don't need it. Should we make a new category for just device states then, or a more general one that could be used for other things too? What do you think?

By: Håkon Nessjøen (haakon) 2010-02-01 03:30:07.000-0600

The reason I sent an additional event for the response, was because I read manager_dbget in main/cdr.c to see how to answer with one response. And it used an additional event, so I thought that was the way to go. I will fix that too.

By: Håkon Nessjøen (haakon) 2010-02-01 13:21:44.000-0600

What about adding a new class:
; deviceinfo - General information about devices. Read-only.

Where we later would also add other data that may be coming from channel drivers about a specific device, not affiliated with calls, like these.

What do you think?

By: Håkon Nessjøen (haakon) 2010-02-02 10:04:00.000-0600

Uploaded a new version with single-response as Olle mentioned. And added a new class "deviceinfo" where these commands lie, and the events come from.

By: Leif Madsen (lmadsen) 2010-02-03 10:19:32.000-0600

Can I delete patches uploaded prior to the newmanager patch?

By: Håkon Nessjøen (haakon) 2010-02-03 10:38:24.000-0600

Yes please :)

By: Olle Johansson (oej) 2010-02-03 12:38:52.000-0600

Looks very good.

Be patient, I'll run some tests and integrate this, but it will take a few days.

By: Olle Johansson (oej) 2010-02-03 12:42:56.000-0600

Sent mail to asterisk-dev just to check that everyone agrees about adding a new class to manager.conf.

By: Håkon Nessjøen (haakon) 2010-02-03 13:11:23.000-0600

A little slap on my own hands. There are some brackets missing on the ActionID lines. According to the coding guidelines.. ;P

By: Håkon Nessjøen (haakon) 2010-02-04 03:52:33.000-0600

New patch with devicestate class instead of deviceinfo, as confirmed on asterisk-dev mailinglist.

By: Corey Farrell (coreyfarrell) 2012-01-03 06:15:41.841-0600

I realize it's a bit late to comment on this bug/patch, but the Get/Set functionality already exists:
Action: Setvar
Variable: DEVICE_STATE(Custom:lamp1)

Action: Getvar
Variable: DEVICE_STATE(Custom:lamp1)

Just bringing this up as there is often duplication between CLI, Dialplan apps and AMI commands.  For dialplan functions we don't have to duplicate to AMI since it can be used as shown above.

By: Matt Jordan (mjordan) 2015-03-14 17:01:07.867-0500

And in Asterisk 13 at least, we also have the [DeviceStateChange|https://wiki.asterisk.org/wiki/display/AST/Asterisk+13+ManagerEvent_DeviceStateChange] event.

As such, since all aspects of this new feature are now in Asterisk, I'm going to close this out as Fixed.