[Home]

Summary:DAHLIN-00072: [patch] Generate VMWI neon pulses from FXS module to light NEON lamp on newer 'non intellegent phone'
Reporter:Alec Davis (alecdavis)Labels:
Date Opened:2009-01-16 15:03:54.000-0600Date Closed:2019-05-31 10:40:34
Priority:MajorRegression?No
Status:Closed/CompleteComponents:wctdm/NewFeature
Versions:2.1.0.3 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk.neon.diff.txt
( 1) bad_neon_ring.jpg
( 2) neon_ring.jpg
( 3) neontest.sh
( 4) wctdm_fix_ONHOOKTRANSFER.diff.txt
( 5) wctdm.c_neon_support.diff5.txt
( 6) wctdm.c_ohttimer_click.diff.txt
( 7) wctdm24xxp_neon.diff.txt
( 8) wctdm24xxp.neon.diff6.txt
( 9) wctdm7109.part2.diff.txt
(10) wctdm7109-neonmwi.diff
(11) wctdm7146.neon.diff2.txt
(12) wctdm7153.neon.diff.txt
Description:Add Message Waiting Indicator method to light NEON lamp on low cost phones with integrated Neon Lamp

Using /etc/dahdi/chan_dahdi.conf keyword mwisendtype=hvac


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

continuation of http://bugs.digium.com/view.php?id=14104 which added fundamental support for multiple MWI methods.

Following Silicon labs AN33. xpp/card_fxs.c already supports neon generation.
Comments:By: Alec Davis (alecdavis) 2009-01-21 12:22:10.000-0600

Implementation details: on a TDM400P at channel 32

/etc/asterisk/chan_dahdi.conf
 .....
 mailbox=2345
 mwisendtype=neon
 channel => 32

<b>If you want this feature please press the "<u>Monitor Issue</u>" button</b>


By: Alec Davis (alecdavis) 2009-01-21 12:22:27.000-0600

uploaded wctdm_neon_support.diff.txt

This adds neon generation support to the wctdm driver.

As this changes the proslics ringer mode between 'neon generation' and 'ring generation' there is some code changes related to fxshonormode that has changed.

I'm sure I've covered the fxshonormode intentions, although
if opermode=AUSTRALIA the following are set
 fxshonormode=1
 boostringer=1

meaning register 21 gets overridden by the later 'if boostringer'


By: Alec Davis (alecdavis) 2009-01-22 03:33:21.000-0600

uploaded wctdm_neon_support.diff2.txt

introduced wctdm_init_ring_generator that sets up proslic registers that are static between neon ring mode and normal ringer mode.

wctdm_set_ring_generator now only set proslic registers that change between modes.

Tested on TDM400P with different FXS modules with proslics Si3210 and Si3215


By: Alec Davis (alecdavis) 2009-01-23 00:53:19.000-0600

Anyone intimately familiar with the Si3210 and Si3215 proslic?

The code is working well so far, requires the linefeed register (Direct Register 64 = 1) set to Forward Active mode before the registers RNGY RNGX etc. can be set. Capture trace uploaded neon_ring.jpg

If the linefeed register is set for Reverse Active (5), and then the registers RNGY RNGX and others are set, both TIP and RING generate opposing trianglar 4HZ sawtooth type waveforms (not pulsed DC anymore), causing a phone's ringer to trip at 4HZ, as can be seen in the uploaded file bad_neon_ring.jpg

The application for this is where there are 2 different phones type on the same line, where one supports LineReversal VMWI, and the other only supports Neon VMWI.

The only way I can see to have a 'reverse polarity neon ringer' is to set the On Hook Line Voltage (register 72) VSGN bit. But IIRC this gave similar results as not setting the Linefeed=ForwardActive before setting registers for neon generation.

Trunk as of a few days ago supports LineReversal VMWI, wctdm_neon_support.diff2.txt adds neon support.

Meantime:
 FSK MWI is virtually untouched, and is supported in parallel with 'lrev' and 'neon'.
 LineReversal and Neon Generation are mutually exclusive. Altough they can both be enabled, 'neon' overrides.

edit 26 Jan: wctdm.c_neon_support.diff3.txt now allows LineReversal and Neon.

edit 10 Feb: Silabs FAE Taiwan confirms that for correct Neon Pulses the Line feed register needs to be in Forwad Active State.


By: Alec Davis (alecdavis) 2009-01-25 02:52:12.000-0600

The anwser to this may be as simple as.

If neon is configured, operate the linefeed in ReverseActive while there are no messages, and when VMWI is required, setting Neon pulse generation will first set the linefeed to Forward Active and generate Neon pulses in the Forward like mode, the phone with only Line Reversal MWI support will presumably also turn on it's light, perhaps modulated at 4HZ.

The above implies, that if 'neon' is enabled, 'lrev' is also enabled.

But not necessarily, but below complicates things.
if only 'neon' is enabled we can operate in Forward mode always.
if both 'neon' and 'lrev' are enabled we need to operate in Reverse as described above, and Forward when VMWI is set.


By: Alec Davis (alecdavis) 2009-01-26 03:52:23.000-0600

please remove wctdm_neon_support.diff.txt and wctdm_neon_support.diff2.txt

uploaded wctdm.c_neon_support.diff3.txt
Which changes the normal linefeed polarity to reversed if 'neon' is enabled, as 'neon' generation requires linefeed to be Forward Active before setting registers.

This allows 'mwisendtype=lrev,neon', the LED on the phone with only LineReversal is modulated if 'neon' is enabled, the Neon on the other phone is pulsed.

The POLARITY macro now has another term to XOR.

Tested on TDM400P with 2 FXS modules, 4 phone types (No MWI lamps, LED MWI, NEON MWI, HV MWI), all with an inline callerid unit.
All functions tested OK including FSK MWI and CallerId operation, at both the phones and CallerId unit.

Further consideration required below:

I wonder should there be a user configurable channel parameter 'reversepolarity' to operate the FXS line in reverse linefeed, as does the global module parameter 'reversepolarity'. The internal variable name would perhaps be 'fxs.user_reversepolarity'. We cannot just use 'fxs.reversepolarity' as this is intended later to be used for 'Line Reversal Alert Signal' type application.

Then if the user configured (in chan_dahdi.conf) 'reversepolarity=yes' or 'mwisendtype=neon' the new internal channel variable 'fxs.user_reversepolarity' would be set, thus operating the linefeed in reverse, unless a message is waiting.

Taking 'user_reversepolarity' even further, for installation ease, consider the case when 'neon' may be configured at a later stage for a 2nd phone, 'lrev' should also set 'user_reversepolarity', that way no switches need to be changed on the customers phones, (some are dip switches, and hard to get at!).


By: Alec Davis (alecdavis) 2009-01-26 04:04:07.000-0600

To do some of this debugging I've been using a script to set registers using fxstest. But it's got a bug, you cannot set indirect registers to a value greater than 255 refer http://bugs.digium.com/view.php?id=14323
By: Doug Bailey (dbailey) 2009-01-27 17:56:26.000-0600

We are presently locking down adding new features to dahdi for a few days.  When I get some bandwidth, I will work on trying to integrate what you have proposed here.
By: Alec Davis (alecdavis) 2009-01-27 18:05:37.000-0600

Just enjoyed helicopter flight at Mt Maunganui. NZ.
Test equipment is 600Km away.

Brakes are on.
By: Alec Davis (alecdavis) 2009-02-01 02:42:00.000-0600

uploaded neon_ring.jpg
Screen capture of good neon pulses at phone jack.

uploaded bad_neon_ring.jpg
Screen capture of incorrect neon pulses at phone jack, this is caused by setting linefeed register to Reverse Active before setting Ring Active/Inactive timers.

uploaded wctdm.c_neon_support.diff4.txt
Applies cleanly to latest trunk as of last night.
Prevent polarity reversals if neon is configured when sending FSK MWI and when going offhook.

uploaded neontest.sh
Uses fxstest to set registers to setup TDM400P to generate neon pulses.
wctdm_ioctl DAHDI_SETPOLARITY exit if not FXS before getting user data.


By: Alec Davis (alecdavis) 2009-02-05 06:47:59.000-0600

please remove the following files:
wctdm_neon_support.diff.txt
wctdm_neon_support.diff2.txt
wctdm.c_neon_support.diff3.txt
wctdm.c_neon_support.diff4.txt

uploaded wctdm.c_neon_support.diff5.txt

Further 'neon' testing revealed that the OnHookTransfer timer 'ohttimer' expired then reset the called extension approximately 6 seconds into a call. I now clear the ohttimer if we are offhook.

The real problem I believe is that 'wc->mod[x].fxs.lasttxhook' should be set to 1 or 5 when the ringing extension answers (goes offhook), but is not being set,
depending on polarity it's left at 2 or 6, which allows the 'ohttimer' to expire, which with 'neon' enabled causes the ring generator to reset.


By: Alec Davis (alecdavis) 2009-02-06 04:50:39.000-0600

dbailey:

uploaded wctdm.c_ohttimer_click.diff.txt

The ohttimer expiring issue, 'click is heard 6 seconds after call is answered' also exists prior to any of our 'lrev' and 'neon' work.

edit: Sunday 10:15pm (NZST)
Although untested for non trunk releases, thus unconfirmed, looking at previous early zaptel releases back to Zaptel wctdm.c release tag 0.8.0 when 'ohttimer' was first introduced, the code hasn't changed much regarding ohttimer, I believe this 'click' is present in all prior releases, and the fix using 'oldrxhook' appears as though it would fix all releases.


By: Alec Davis (alecdavis) 2009-02-11 03:57:48.000-0600

uploaded wctdm24xxp_neon.diff.txt
which includes wctdm24xxp/base.c and wctdm24xxp/wctdm24xxp.h diffs

Unable to test but compiles, hopefully I managed to get all relevant changes.
This is based on the wctdm.c_neon_support.diff5.txt
By: Alec Davis (alecdavis) 2009-02-11 17:45:29.000-0600

Doug:
Are you able to take a look at and test wctdm24xxp_neon.diff.txt?
By: Doug Bailey (dbailey) 2009-02-11 20:39:54.000-0600

My biggest concern is the bandwidth available to set the hvac mode.  (As well as switching out of it.)  Each register access to the slic IC has a sizable sleep element with it. The indirect registers are even more sizable.  

Let me try to get some calculations worked out to see what the time delay is to place the device into and out of the hvac mode
By: Doug Bailey (dbailey) 2009-02-25 13:49:01.000-0600

I was looking at the wctdm24xxp patch (finally) and I noticed a couple of items.  

The calls to wctdm_setreg from within the isr (wctdm_set_ring_generator_mode is called from wctdm_isr_misc) need to be changed.  The calls being made call for sleep, which cannot be done within the interrupt context.  I need to look at what can be done to pull the update of those registers into something that is interruptible.  I will try to see if I can work that functionality into the command queue.
By: Alec Davis (alecdavis) 2009-02-25 18:55:57.000-0600

In wctdm calls to wctdm_setreg were already being done from the isr, when ohttimer expires. Does the command queue need to be implemented in wctdm, or does it already exist?

Happy to test wctdm version when your ready.
By: Doug Bailey (dbailey) 2009-02-25 20:13:25.000-0600

One of the biggest difference between the wctdm and wctdm24xxp is the manner in which the slic and daa registers are accessed.  The wctdm24xxp accesses them via voicebus which has its own layer of overhead.  Therefore, the wctdm24xxp is more involved in making these changes.
By: Alec Davis (alecdavis) 2009-03-04 04:26:22.000-0600

Doug: Any progress?

I need to know if I need to, and how best to, handle the wctdm_set_ring_generator_mode another way, infact the calls to wctdm_setreg that happen from within the interrupt.

Can't we just set a flag, that gets monitored by a do_thread?
Or if we can afford 1 or 2 writes per interrupt, develop a statemachine to drive the line feed registers and ringer to the correct state, I guess that's what the command queue does for the wctdm24xxp.

After all, the ringer mode doesn't need to change in a split second.
By: Alec Davis (alecdavis) 2009-04-29 03:56:58

I should have my hands on a TDM800P with 8 FXS in next few days.

Was any progress made with the concerns of setting HVAC mode (90V DC pulses)? Particularly from within an interrupt.

Will also verify whether OHTTIMER click bug exists.

Alec
By: Doug Bailey (dbailey) 2009-06-01 11:14:17

Sorry for the delay in getting back to you.

Once we get dahdi V2.2 released, I will roll your changes to wctdm.c into dahdi
trunk.

I am still waiting on the command queue changes to be made and committed to the
wctdm24xxp driver before I try to bring the neon vmwi into that driver.  (The
command queue changes will be similar to those made in the wcte12xp driver.)
There are a couple of issues that are holding that up, further delaying the neon
changes.
By: Alec Davis (alecdavis) 2009-07-16 16:06:52

Doug:
Any chance of getting Neon into wctdm, or even for wctdm24xxp.

I can now test both, anything I need to do?
By: Alec Davis (alecdavis) 2009-09-09 05:18:56

Doug:
Did the command queue changes happen, to allow Neon progress for the wctdm24xxp?
By: Shaun Ruffell (sruffell) 2009-09-09 10:31:23

Alec:
This is in my court as well....but I'm not sure I'll be able to get to it this week.
By: Doug Bailey (dbailey) 2009-09-11 16:53:48

Alec,
The wctdm7109-neonmwi.diff patch is your patch cleaned up against dahdi trunk at commit 7109.  I've done some cursory testing and will be testing more early next week.

We will probably go ahead and commit this to trunk soon.  The wctdm24xxp implementation will have to wait until the command queuing gets cleaned up.
By: Alec Davis (alecdavis) 2009-09-14 06:06:15

Tested tonight cleaned up patch, after gettting hardware together with both proslics Si3210 and Si3215.

Tested only with neon phones, don't have a phone capable of showing line reversal phone at present.

Still working as expected :)
By: Alec Davis (alecdavis) 2009-09-15 05:46:32

Doug:
wctdm7109.part2.diff.txt applies after wctdm7109-neonmwi.diff has been applied.

Although code tested OK, after reading the changes, I noticed in 'wctdm_ioctl' the ONHOOKTRANSFER idletxhookstate was SLIC_LF_OHTRAN_xxx instead of SLIC_LF_ACTIVE_xxx

This was fixed in trunk 'R6941' DAHLIN-119 a while ago. But as I hadn't maintained this neon patch, the change got reverted.

also additional cleanup in 'set_vmwi'.

compiles, but untested.
By: Alec Davis (alecdavis) 2009-09-16 06:07:30

uploaded wctdm7146.neon.diff.txt

combined wctdm7109-neonmwi.diff and wctdm7109.part2.diff.txt and applied against R7146.

Tested OK with both proslics.
By: Alec Davis (alecdavis) 2009-09-16 06:13:23

Doug:
Regarding your concerns about the setting of the ring generator to neon mode in the interrupt, for the wctdm24xxp based cards.

You'll have noticed if you have a message that the Neon comes ON when you go OnHook, then goes OFF for the FSK spill, then back ON after the ohttimer expires.

The above seems a bit messy for the average user.

How about, we set the DAHDI_VMWI at the end of the state machine ' mwi_send_process_buffer', and remove the ringgenerator setting in the interrupt.

So the states become
  1. RPAS SEND if configured (mwisendtype=rpas)
  2. RPAS PAUSE
  3. MWI_SEND_SPILL if configured (mwisendtype=nofsk could prevent it)
  4. DAHDI_VMWI SET if configured (mwisendtype=lrev,neon)
By: Alec Davis (alecdavis) 2009-09-17 05:20:16

please remove:
wctdm7146.neon.diff.txt

uploaded:
wctdm7146.neon.diff2.txt and asterisk.neon.diff.txt

Neon implementation without calling ring generator from within the interrupt.

chan_dahdi, now calls the VMWI ioctl after the MWI FSK has been sent, all from 'mwi_send_process_buffer'.

Doug, Let me know if this implementation is satisfactory for the wctdm24xxp?
By: Doug Bailey (dbailey) 2009-09-17 11:04:39

Alec,
From what I can see of the latest patch, any time the channel goes into OHT
mode, the neon MWI signaling mode is lost.  (Hence the need to place the ioctl
call after the completion of the MWI fsk spill.)  Though I prefer to get the
register setting calls out of the ISR, I also prefer not having to order the
calls to the VMWI and OHT ioctls within chan_dahdi.  Given that I think that not
ordering the ioctl calls is more important, I prefer the original
wctdm7146.neon.diff.txt patch.

As for the wctdm24xxp, we are trying to restructure the command queue to the
slic chips.  When we get that restructured, the calls for setting the registers
for neon MWI should be straight forward.

Doug
By: Doug Bailey (dbailey) 2009-09-17 13:53:05

I've attached wctdm7153.neon.diff.txt which is wctdm7146.neon.diff.txt with some cosmetic trimming as dictated by checkpatch (though the ISR code was not touched) and the addition of a define that dictates the MWI pulse width.  (I had a system that I was testing that needed to see the MWI pulse active for > 100 mS)  

Looks good from my testing.
By: Alec Davis (alecdavis) 2009-09-18 04:03:00

One of the underlying reasons to order the sending of the MWI indication method is so that alternative alerting methods can be used before the FSK.

I started a bug note ASTERISK-13694 but have not yet started implementatio.

Basically it is to support the following:
  DT-AS: Dual Tone Alerting Signal.
  LR+DT-AS: Line Reversal + Dual Tone Alerting Signal.
  RP-AS: Ring Pulse Alerting Signal. Is already done with mwisendtype=rpas
By: Doug Bailey (dbailey) 2009-09-18 10:31:28

I agree that those can be implemented by further enhancements to the mwi state
machine.  I would add those as part of issue 014602 as you mentioned.

For this issue, I suggest that we commit wctdm7153.neon.diff.txt to provide neon
MWI from the wctdm driver.  We will then make the necessary changes to the
wctdm24xxp driver to get it to support neon MWI after we revamp the voicebus
command queue structure.

The overall goal is to provide support for the following MWI generation:
- Neon pulses (Possibly with options for different pulse durations)
- line reversal
- fsk only
- fsk preceded by Ring Pulse Alerting Signal (RP-AS)
- fsk preceded by Dual Tone Alerting Signal (DT-AS)
- fsk preceded by Line Reversal + Dual Tone Alerting Signal (LR+DT-AS)
By: Alec Davis (alecdavis) 2009-09-18 17:00:15

It's working for me with Neon only phones when mwisendtype=neon.
Also working when mwisendtype=lrev, with LED phones.

But back at ~98724 I thought I left it where line reversal and neon worked together.

The reason is that the Si321x Proslics in Neon generation mode only work correctly in ACTIVE_FWD mode, meaning the non-message state of the line should be ACTIVE-REV, this will also cater for LineReversal VMWI phones.

The result is no matter what type of phone is plugged in, Neon or LineReversal VMWI, that the VMWI will still indicate messages.

But exclusively using one type or the other, it's working well.

I'm happy with this, If I find customers with the requirement to intermix phones, then we'll address it later.
By: Alec Davis (alecdavis) 2009-09-21 15:51:34

Doug:
Regarding using Neon and LED phones together, Last night I retested dahdi from around february 3rd 2009 (IIRC R5963), and although I thought both types gave the appropriate indications, it appears not, they work independantly for the mode that they've been configured.

Lets get this commited for testing, and in the near future, get both types working together. Sure it would be nice to have both, but the work required will further delay progress.
By: Digium Subversion (svnbot) 2009-09-21 17:21:21

Repository: dahdi
Revision: 7187

U   linux/trunk/drivers/dahdi/wctdm.c

------------------------------------------------------------------------
r7187 | dbailey | 2009-09-21 17:21:20 -0500 (Mon, 21 Sep 2009) | 12 lines

wctdm:  Add ability to generate neon MWI from fxs ports

Add a mode of operation for fxs ports to use the slic's ring generator to
produce neon MWI pulses.  Patch was implemented bty Alec Davis.  

(issue DAHLIN-72)
Reported by: alecdavis
Patches:
     wctdm7153.neon.diff.txt uploaded by dbailey (license 819)
Tested by: alecdavis, dbailey


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

http://svn.digium.com/view/dahdi?view=rev&revision=7187
By: Alec Davis (alecdavis) 2009-09-22 03:24:21

uploaded wctdm_fix_ONHOOKTRANSFER.diff.txt

Adds the missing break in the DAHDI_ONHOOKTRANSFER wctdm_ioctl.
Fixes the ability to interchange Neon and LED (LineReversal) type phones on the same FXS line, and have the correct VMWI indications.
By: Digium Subversion (svnbot) 2009-09-22 09:06:03

Repository: dahdi
Revision: 7194

U   linux/trunk/drivers/dahdi/wctdm.c

------------------------------------------------------------------------
r7194 | dbailey | 2009-09-22 09:06:02 -0500 (Tue, 22 Sep 2009) | 12 lines

wctdm: Add missing break

A break was missing that caused DAHDI_ONHOOKTRANSFER ioctl call to fall into
DAHDI_SETPOLARITY ioctl call.

(issue DAHLIN-72)
Reported by: alecdavis
Patches:
     wctdm_fix_ONHOOKTRANSFER.diff.txt uploaded by alecdavis (license 585)
Tested by: alecdavis


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

http://svn.digium.com/view/dahdi?view=rev&revision=7194
By: Alec Davis (alecdavis) 2010-11-17 00:42:01.000-0600

Others are catching on, Grandstream GXW4024 HW Rev. 0.4, lastest beta 1.0.1.63 now supports NEON MWI.

Just to recap, NEON VMWI works with the wctdm driver (obsolete hardware), but not yet the wctdm24xxp (current hardware).
By: Shaun Ruffell (sruffell) 2011-03-14 15:08:36

alecdavis: I've made the vast majority of the changes to the wctdm24xxp to support the "cmdlist".  I hope to have them committed to trunk soon, but I'll keep rebasing a set of changes on git://git.asterisk.org/team/sruffell/dahdi-linux (on the wctdm24xxp-cmdlist branch).

Anyway, just giving you a heads up in case you wanted to look at adding these patches on top of that branch.

(webview: to the branch in question: http://git.asterisk.org/gitweb/?p=team/sruffell/dahdi-linux.git;a=shortlog;h=refs/heads/wctdm24xxp-cmdlist )


By: Alec Davis (alecdavis) 2011-03-16 05:43:22

wctdm24xxp_neon.diff2.txt

shaun: based off wctdm24xxp_neon.diff.txt, compiles only!
By: Shaun Ruffell (sruffell) 2011-03-16 16:42:58

After talking to you on IRC, I've mirrored that git tree out to a team folder at:

http://svn.asterisk.org/svn/dahdi/linux/team/sruffell/wctdm24xxp-cmdlist
By: Alec Davis (alecdavis) 2011-03-18 18:30:38

wctdm24xxp_neon.diff6.txt

current working version against http://svn.asterisk.org/svn/dahdi/linux/team/sruffell/wctdm24xxp-cmdlist