CM11A - adding new lights...

[nosilla99]

Nosilla -

I chased that for a while too, but then I noticed this:

Code: [Select]

#ifdef WIN32

so there is another IsReadEmpth() method used by linux (look further down the page)

Thanks,

I guess trying to read and edit source code on my TV is a bad idea 

NOS

[jondecker76]

Don't feel bad - i didn't notice it until I did a debug statement in there!

We still have some work to do to get this interface working like it should

[nosilla99]

Yes although the key issue is now fixed so once the latency is resolved it should be quite usable 

NOS

[jondecker76]

Can you get on IRC?

I think the problem lies in IsReadEmpty()

[jondecker76]

I think I can see what is causing the latency..

Code: [Select]

if (serprt.IsReadEmpty()) {
unsigned char resp;
if(serprt.Read((char*)&resp, 1, CM11A_READ_TIMEOUT) != 0)
{

The if(serprt.IsReadEmpty) is checking to see if data is available to read.
Most of the time is should be empty. But by removing the "!", it is checking all the time for the most part.
Then it falls down to if(serprt.Read((char*)&resp, 1, CM11A_READ_TIMEOUT) != 0)
Here it tries to read the data (in the previous step we already found that it should be empty most of the time) that isn't there, until the CM11A_READ_TIMOUT is reached (15 seconds)
This is causing the delay I think.

I'm going to poke around in the IsReadEmpty() a bit more...

[nosilla99]

Yes you are correct a simple fix could be to create another constant called CM11A_POLL_TIMEOUT with a value of 500 and use this in the line you stated above although the fact that this routine is always called points to an error in the ser.IsReadEmpty() not correctly identifying if the RI indicator is being toggled so as you suggest that would be a better place to focus effort as the simple fix will increase CPU usage by the CM11A code.

Good Luck

NOS

[nosilla99]

@jondecker76

I have implemented a new function in serial.cpp named IsRngSet() which checks for the Ring Indicator as specified in the CM11A protocol definition.  By calling this function instead of IsReadEmpty() the CM11A code works properly with no other changes required.

The latency is approx 1 second for LinuxMce commands and externally trigered events are shown in the CM11A logs as expected.

I have only had one instance of the dreaded checksum error in the last hour after lots of testing so I am happy that the approach is a correct one, but still believe that the sendpacket() routine should also check for a 0x5A response and deal with it on the rare occasions that it crops up.

The function is listed below, you will also have to add a declaration in SerialPort.h have a try and let me know how you get on

Code: [Select]

/** Nosilla99 Added function IsRngSet() to check for incomming data from CM11A */
bool CSerialPort::IsRngSet()
{
int flags = 0;

pthread_mutex_lock( &d->mutex_serial );
int iRNG = ioctl( m_fdSerial, TIOCMGET, &flags );
pthread_mutex_unlock( &d->mutex_serial );

if( -1 == iRNG )
{
// bad serial device
return false;
}

if( !(flags & TIOCM_RNG) )
{
return false;
}

return true;
}

Best Wishes

NOS
 

[jondecker76]

Excellent Job!  I won't be able to help with testing until tomorrow as I am at work for another 14 hours - but it looks just like I expected it would after our talk on IRC.

***edited -there was a big error in my logic...****
I see what you are saying about dealing with the 0x5A from within sendPacket() now, and after looking at the source some more I agree. The easiest (and most direct) way of dealing with it would be to just totally discard any 0x5A polls (don't send the 0xC3 ACK either). The CM11A will send its 0x5A every second until we respond with the 0xC3 ACK, so just discarding them while in sendPacket() all together should be fine, as once we are finished with sendPacket(), the correct logic block will catch the 0x5A poll and deal with it accordingly. Once this is done, it should be 100% stable!

Great job once again!

[jondecker76]

What do you think about something like:

Code: [Select]

if(pport->Read((char*)&resp, 1, CM11A_READ_TIMEOUT) == 0) {
LoggerWrapper::GetInstance()->Write(LV_STATUS, "No response from CM11A device.");
return -1;
} else {
LoggerWrapper::GetInstance()->Write(LV_STATUS, "Got response: %x from CM11A.", resp);
if(resp == CM11A_CLOCK_REQ) {
LoggerWrapper::GetInstance()->Write(LV_STATUS, "CM11A requested clock SET UP", resp);
SendClock(pport);
return -1;
} else if(resp == CM11A_INTERFACE_CQ) {
//Do a log entry here maybe? Not really needed, but maybe a message like "Data ready to be received, we will deal with it after sending..."
return -1; //it will try again...
} else { 
if(resp != chksum) {
LoggerWrapper::GetInstance()->Write(LV_CRITICAL, "Bad checksum received (send:%x, recieved:%x).", chksum, resp);
return -1;
}
}
}
?


Once this is ironed out (shouldn't be long now) I'm going to dust off my CM15A (USB version) and see what we need to do to get it to work, and to get it to plug and play!

[nosilla99]

@jondecker,

unfortunately I believe we have to deal with any incomming data before the CM11A will allow us to send commands so the current CM11A logic needs recoding.

A dirty fix would be to carry out a poll of the CM11A interface in the SendPacket() function prior to issuing addresses and functions, if there is data waiting then we can read it and deal with it our just discard the event.

Discarding the event is a bad thing to do as it could have safety implications such as a security sensor being tripped, hence we probably need to completely re-work the logic.  I am going to see if I can find an RS232 breakout box as I also want to monitor the Ring Indicator to ensure it stays high when there is incomming data.

In terms of the CSerialPort::IsRngSet() it is generic and I only added a comment regarding the CM11A for my own documentation, but I will write a more general routine to check for any serial port status bits as requested.

Don't work too hard 

Hopefully catch you later on IRC

NOS



 

[jondecker76]

Are you sure that you have to deal with incoming data before the CM11A will send commands? Have you tested this?
An easy way to test this is to change devicepoll.cpp so that the 0xC3 ACK is never sent (so that the 0x5A poll will transmit every second) once it receives data. Then, once in this state, try to send some commands.. If it works, then we are home free.

Also - by discarding, i just mean not responding to the 0x5A. We would just not respond to the poll until after we send our address/function. All of the data will still be there waiting - nothing would be lost, and the poll will continue to fire every 1 second until we do respond to it with the 0xC3 ACK. Therefore, if we can prove that we can send address/functions while the CM11A is doing its 0x5A poll, then this is the fix. If you don't get to it before me, i will test that part when I get home in an hour or two.

I also would like to know how long the RI stays high. My guess is that it will stay high until the CM11A receives the 0xC3 ACK from us, but it would be nice to know for sure

[nosilla99]

@jondecker

The problem is that unless you deal with the incomming data, you will not receive any valid checksum responses, I also know that after allowing the send requests to timeout i.e. fail the ring indicator is no longer set so we are definitely going to have to fix the CSerialPort::IsReadEmpty() function, although changing this could effect any other code which also uses this function so we will have to identify all other bits of software which call it 

I will try treating 0x5A as a valid checksum just to verify that the CM11A won't process requests if it has data waiting and let you know the result

NOS

[jondecker76]

0x5A is a valid checksum for x10 address header G1 ( (0x56+0x04)&0xFF)

ok, i'm just messing around..
But, will be interesting to hear your results!

[nosilla99]

@jondecker

I have finished testing the CM11A code and the my worst thoughts look to have come true

1. It is not possible to send commands to the CM11A if it has data waithin in its buffer to be processed

2. It is possible for it to flag that there is data waiting even when you are halfway through issuing commands to the CM11A

3.  The logic in the CM11A code is fundamentally flawed and needs a complete re-write

Basically before sending packets you must check to see if the CM11A has data waiting and if so you must do something with it, fortunately most of the code is fine it is just a logic problem so we will need to breakout the CM11A reading code from _Run() into a sub function and during sending packets if data is found to be waiting call the new sub function to process it.

I don't think there is an excessive amount of work required, just some restructuring of the code and fixing the CSerial::IsReadEmpty() function

On a positive note the current code using the ring indicator works well if you don't have things like motion detectors(it is these which are causing events to be received in the middle of sending commands from LinuxMCE i.e I wave the motion detector around and select lights on or off in the orbitor is a sure way to break things 

NOS

[nosilla99]

I have now recoded  and fixed the logic problems with CM11A 

All testing has proved successful with none of the old 0x5A checksum problems which caused a failure in the code

I still need to amend the code to correctly fire off DCE messages where appropriate, although this is not a major issue as the polling never used to work 

NOS