Going nuts! What am I doing wrong?

photomankc

This code should, as I understand things, work. But the whole function is acting crazy. No matter what value I place in the array, even if I directly assign it in the function, I never get that going out the bus, its always a fixed value that has no relation almost like It sent the address and not the value. mailbox.buffer is a pointer to type uint8_t. So if I create an array of those named bytes and give that to the stucture that should work yes?

int i2cTXWordReg(I2C_STATE *dev, int address, uint16_t reg, uint8_t rcnt, uint16_t wd)
{
    // write 0x1A2B -- high byte first;
    uint8_t bytes[2];
    bytes[1] = 0x2B;
    bytes[0] = 0x1A;  
    

    dev->mailbox.hdr        = (address << 1) | I2C_WRITE;
    dev->mailbox.buffer     = bytes;
    dev->mailbox.count      = 2;
    dev->mailbox.reg        = reg;
    dev->mailbox.reg_count  = rcnt;
    dev->mailbox.cmd        = I2C_CMD_REG_SEND;
    
    while (dev->mailbox.cmd != I2C_CMD_IDLE)
        ;

    return dev->mailbox.sts == I2C_OK ? 0 : -1;
}

Thing is that any function variable I create acts the same. I get some bogus value and not the value I put in the variable. This for instance should produce a write of 0x0000 but instead gave me 0x280C.

int i2cTXWordReg(I2C_STATE *dev, int address, uint16_t reg, uint8_t rcnt, uint16_t wd)
{
    uint16_t  temp = 0; 
    
    dev->mailbox.hdr        = (address << 1) | I2C_WRITE;
    dev->mailbox.buffer     = (uint8_t*)&temp;

}

This works fine and sends the bytes I call it with but in the reverse order of what I need them to be:

int i2cTXWordReg(I2C_STATE *dev, int address, uint16_t reg, uint8_t rcnt, uint16_t wd)
{  
    dev->mailbox.hdr        = (address << 1) | I2C_WRITE;
    dev->mailbox.buffer     = (uint8_t*)&wd;

}

Then interestingly this has no effect what-so-ever despite the fact that as I understood it I should have passed wd by value so that I should be able to alter the value. The result is that it sends the bytes the function was called with completely ignoring the fact that I reset the variable to 0? WTH?

int i2cTXWordReg(I2C_STATE *dev, int address, uint16_t reg, uint8_t rcnt, uint16_t wd)
{
    wd = 0; 
    
    dev->mailbox.hdr        = (address << 1) | I2C_WRITE;
    dev->mailbox.buffer     = (uint8_t*)&wd;

}

I'm totally stumped. So long as the buffer is one that is not created in the fuction then the write code in the cog driver works perfectly. It's when I try to use a buffer that is created within the function that it's writing out the garbage values and I can't figure out why at all.

David Betz

Can you post your entire program? Also, which memory model are you using? LMM?

photomankc

XMMC. Let me try to unwind some of the Smile I've added in trying to track this down and I'll package it up and post it. There is something going on with the while (dev->mailbox.cmd) line. It's a scope issue because the function is exiting before the bus has gotten around to writing the bytes and therfore the variables are going out of scope before the write is complete.

I added a small delay in the while loop and suddenly the low byte made it out correctly but the highbyte was gibberish. Added double the delay and bingo the bytes all made it out. Used a printf to verify that the dev->mailbox.cmd was not becoming 0 before the while loop and confirmed that. So why then was it just plowing on through and returning anyway? I turned off all optimization and then the code works with or without the delay code within the while loop. For some reason the optimizer is deciding that while loop is not required and seems to be snatching it out during compile time. I guess I see that, but a while loop to poll something is used all over so why just here.

I'll get the code up tonight.

ersmith

photomankc wrote: »

XMMC. Let me try to unwind some of the Smile I've added in trying to track this down and I'll package it up and post it. There is something going on with the while (dev->mailbox.cmd) line. It's a scope issue because the function is exiting before the bus has gotten around to writing the bytes and therfore the variables are going out of scope before the write is complete.

Did you remember to declare dev->mailbox.cmd to be volatile? That's a pretty common cause of problems, and such problems usually go away when you turn off optimization (as in your case).

Eric

photomankc

Ugh..... yep that's it. The mailbox pointer was declared as volatile within the I2C_STATE struct but mailbox.cmd itself was not. Makes sense now. The compiler sees a loop with a variable that looks like it will never satisfy the condition for the while in that scope and rubs it out. I'm just surprised that it would leave behind the code from inside the loop? How would that ever not be undesired? That's why adding a delay in the loop let one byte leak out because the delay got left behind even though the loop it was in was nixed.

jazzed

photomankc wrote: »

Ugh..... yep that's it. ... I'm just surprised that it would leave behind the code from inside the loop? How would that ever not be undesired?

The volatile keyword must be used to let the optimizer know that the code/data should not be optimized away - this is a standard embedded coding practice with C.

If you never use optimization this is not a problem - someone else using your code may stumble over it though.

photomankc

jazzed wrote: »

someone else using your code may stumble over it though.

That's exactly what happened here. The declarations of the structures are straight out of propboe I2C code.

photomankc

Well,

Now it's the opposite problem. If I compile without optimization the code dies at line 46-48. Hangs and never comes back. If I set it to -Os then it will run. I've been having a great deal of issues when passing the instantiated I2C object down the chain as a reference. Earlier it would die just like this if I tried to perform any read or write on the accel/gyro/or compass objects from inside the MiniIMU9 constructor. So accessing them through members of the MiniIMU9 seems to trigger the issues but I don't understand what the issues are.

I need to be able to share the bus with numerous other objects as that was the point of the exercise. The correct address for the bus is getting all the way down to the accel/compass/gyro for them to store but somehow it's falling apart otherwise. I've attached the project if anyone has time to waste looking at my mess.

Tor

photomankc wrote: »

Well,
Now it's the opposite problem. If I compile without optimization the code dies at line 46-48.

Note that unless propgcc does things differently from other compilers you'll have to explicitly specify "-O0" (oh-zero) to turn off optimization - it's not enough to simply leave out any -O option.

-Tor

photomankc

I'm doing the builds through SimpleIDE so it is putting -O0 in the options as it reports them on the status pane. And the difference is -O0 results in the program rolling off the rails where everything works fine with -Os. I've put printf()'s all the way down the call stack and they all report the same address for the object that I pass them so I can't see what's the issue. Hard to say when this started though since I don't run it at each optimization level very often.

jazzed

What is your propeller-gcc version ? There was a recent bug that caused COGC programs to not work with XMMC. Please use the latest version 0_3_4_xxxx.

photomankc

>c:\propgcc\bin\propeller-elf-gcc -v

Using built-in specs.
COLLECT_GCC=c:\propgcc\bin\propeller-elf-gcc
COLLECT_LTO_WRAPPER=c:/propgcc/bin/../libexec/gcc/propeller-elf/4.6.1/lto-wrappe
r.exe
Target: propeller-elf
Configured with: ../../propgcc/gcc/configure --target=propeller-elf --prefix=/op
t/parallax --disable-nls --disable-libssp --disable-lto --with-pkgversion=propel
lergcc_v0_3_5_1523 --with-bugurl=http://code.google.com/p/propgcc/issues
Thread model: single
gcc version 4.6.1 (propellergcc_v0_3_5_1523)

photomankc

Noticed something happening that should not have been. I changed the cog startup code to time out even if a cog never indicates that it started. So why the hang the moment I start the bus? Obvious in my zip file was a logic error, I never incremented the timeout loop counter (n) so I added the missing n++. Still it hangs. Added a printf to print the value of n. Suddenly it does work. Add a long delay instead of printf, it doesn't work. On a lark I did this:

    // Start the cog according to the method needed for LMM/XMM memory models
    #if defined(__PROPELLER_XMMC__) || defined(__PROPELLER_XMM__)
        int size = _load_stop_I2CDriver_cog - _load_start_I2CDriver_cog;
        unsigned int cogbuffer[size];
        // memcpy does bytes, so we copy numbytes * 4 = num ints.      
        memcpy(cogbuffer, _load_start_I2CDriver_cog, size<<2);
    #else
        int *cogbuffer = (int*)_load_start_I2CDriver_cog;
    #endif
    
    m_dev.cog = cognew(cogbuffer, &init);   

    volatile int n;
    while (m_dev.mailbox.cmd != I2C_CMD_IDLE)
    {
        if (n > 254)
        {
            m_ready = 0;
            return;          // Timeout waiting after ~2.5ms           
        }
        pollDelay();        // Wait 10 uSecs.
        n++;
    }

Adding a volatile to the local integer (n) declaration in the same function it's used in and my program terminated correctly if I commented out the cog start code to force a failure. Why on earth would I need volatile to keep it from eating a variable in the same function? I'm feeling a little uncomfortable that it's rubbing out my conditions in places I sure would not expect it to do so in.

jazzed

Hmm. Guess i'm behind on versions. Sorry about that.

So, in I2CDriver main, mailbox->cmd gets set to IDLE at startup (after you've set it to INIT).
Then I2CDriver main waits for something other than IDLE. So ....

Shouldn't you wait for IDLE to happen rather than not IDLE after cognew?
Since the first INIT is basically ignored, set INIT and wait for IDLE again.

That's the way I read it. Maybe I missed something though.
--Steve

photomankc

IDLE is what indicates the cog has taken over. The command is set to INIT when we first start and the COG will set it to IDLE once it enters its dispatch loop so we know the cog is running and is ready for a command now.

jazzed

Ok, got it.

What happens if you move the mailbox m_dev out of the I2C class to a global variable?

BTW, I can't explain the other behavior. Someone else will have to answer that one.

photomankc

Hmm, I'm not having any luck finding a way to declare it outside the class where it doesn't barf up 4 pages of errors.

jazzed

photomankc wrote: »

Hmm, I'm not having any luck finding a way to declare it outside the class where it doesn't barf up 4 pages of errors.

Scope errors maybe? Perhaps m_dev should be a pointer in the class and get initialized with an i2c.setMailbox() call. I don't think we should be using the stack for this mailbox interface (i2c instance is on the stack). Give me some time and I'll make a similar project to see what's happening. I have a similar project that uses PASM cogs that works fine, but not cogc (COGC stack related?) - should be easy to convert. Too tired to do this today ....

photomankc

Take a break. I feel the same. Been a long day and I'm not sure I'm any more use fighting this tonight. Thanks for taking a look!

photomankc

Ok,

I've made the change to make the I2C_STATE structure a global... finally figured that one out. That seemed to work for a while so I went on and made a few changes dealing with computing some values from the sensors and then after adding in some simple code to do some subtraction it came unglued again. The prop is not just locking up, it's executing data I'm sure as the QuickStart LEDs will start to light up now and then after the display quits and every now and then it may cause the main program COG to restart. I checked this out and the code should run on any QuickStart even minus the I2C connections. It will just mistakenly believe the device has ACK'd and the clock-stretch timeouts will cuase it to read all 0's so the program should run and start a loop printing values of all zeros from the sensors.

At -Os the zipped project will run and give you output from the loop. To create the failure uncomment the correction factor subtraction in MiniIMU9::GetAccelVector()

vector MiniIMU9::GetAccelVector()
{
    vector result;
    int16_t accelData[3];
    if (m_accel.ReadAllChannels(accelData) == 0)
    {
        result.x = accelData[0] /*- m_accelCor.x*/;
        result.y = accelData[1] /*- m_accelCor.y*/;
        result.z = accelData[2] /*- m_accelCor.z*/;
    }
    else
        result.x = result.y = result.z = -1.0;

    return result;
}

Once you uncomment those subtractions the whole thing crashes at -Os. Now leave them in and set the project to -O0 and once again it will run just fine.

I guess I'm done for a while on this. One of the play time sessions running around in memory must have got some actual bytes out to the reserved values in the Magnotometer because it's now heavily negative biased and was not that way yesterday afternoon. I can probably correct in software later on but for now I'm not realy confident in going much further. Consequences of some of tmy robots I/O lines and random writting to OUTA could be smoke.

jazzed

Seems you've run into a limit of XMMC mode. With XMMC mode, all global data and stack are in HUB RAM. Additionally, as it stands the XMM cache takes 8KB, so only 24KB are available for data and stack.

I can't seem to get past the first cognew on 3 different XMMC capable boards including Quickstart

Changed the board to C3 and memory model to XMM-SPLIT, and the program runs fine with or without the comments mentioned.

I've tried reducing the Cache size for XMMC on Quickstart and C3F, but that doesn't seem to have any affect. That doesn't void changing to memory model XMM-SPLIT for boards with such support, but it does call into question how the cache driver seems to get loaded.

Thanks,
--Steve

photomankc

Wow, I'm a bit stunned I've exhausted 24k of data already. I guess I need to look at the memory map more closely. Does the non-simple printf drag in a good deal of data allocation too? I just can't see where the stuff I have added would approach 24k.

So the I2C_STATE variable that is global gets an address of 0x97C (2428). I figure that is created early in the program. Then I create a few integers and the first object I make in main is the I2c bus object and it has an address of 5f58 (24408). As I understand it the stack should start at the top and grow down, and I wouldn't think the addresses are random so it *looks* like before I even start to move that 24K is chewed up.

photomankc

So this program:

#include <stdio.h>

int main(void)
{
    int   count = 0;
    
    printf("%d\n", count);
    
    while(1)
    ;
    
    return 0;
}

Using Simple Printf Produces a 21KB executable when compiled as C++. If compiled as C then it's only 7KB. I still cant figure out what is filing up 20+KB of RAM in my program.

David Betz

photomankc wrote: »

So this program:

#include <stdio.h>

int main(void)
{
    int   count = 0;
    
    printf("%d\n", count);
    
    while(1)
    ;
    
    return 0;
}

Using Simple Printf Produces a 21KB executable when compiled as C++. If compiled as C then it's only 7KB. I still cant figure out what is filing up 20+KB of RAM in my program.

Have you looked at the map file? That should tell you exactly where everything is placed and how much memory it takes.

photomankc

Here is the map file. I'm not at all familiar with GCC's mapfile output so this looks largely like a giant soup of gibberish to me. Best I can guess out of that is that the data that is known about at compile time is ending at about 0xa88. I see the global I declare for the I2C_STATE structure at 0x988 and that is just where my Serial output says that it is addressed. The very first object declared in main is at 0x5F40. I don't see where that is in the map file. Is there a reference to how to read the map output?

jazzed

photomankc wrote: »

Here is the map file. I'm not at all familiar with GCC's mapfile output so this looks largely like a giant soup of gibberish to me. Best I can guess out of that is that the data that is known about at compile time is ending at about 0xa88. I see the global I declare for the I2C_STATE structure at 0x988 and that is just where my Serial output says that it is addressed. The very first object declared in main is at 0x5F40. I don't see where that is in the map file. Is there a reference to how to read the map output?

The GCC map file is listed by linker section. There is a perl script that makes it prettier on propgcc.googlecode.com assuming you have perl installed - simple ide doesn't install perl. Usually the cog images are last in the image. SimpleIDE search works on the map file .... As I recall, the last (highest address) data symbol is heap_start ... there is a bunch of .debug sections in there for some reason though.

When I was experimenting with cache sizes I forgot to change the cache-size in the .cfg file. I'll have to look at it again.

The 0x5F40 number is definitely in the stack region. More exploring to do ....

David Betz

photomankc wrote: »

Here is the map file. I'm not at all familiar with GCC's mapfile output so this looks largely like a giant soup of gibberish to me. Best I can guess out of that is that the data that is known about at compile time is ending at about 0xa88. I see the global I declare for the I2C_STATE structure at 0x988 and that is just where my Serial output says that it is addressed. The very first object declared in main is at 0x5F40. I don't see where that is in the map file. Is there a reference to how to read the map output?

I may have mislead you by telling you to look at the map. I ended up doing "propeller-elf-objdump -p" to get the sizes of the program table entries and added up the stuff loaded at zero (hub memory) and got about 6.3k so I don't think that you're using too much data space.

David Betz

I'm wondering if one of the problems here is with the cogc driver code. I notice that you use _NATIVE for non-leaf functions. I thought it only worked for leaf functions. We really need a better description of COG mode! You might want to remove the _NATIVE declarations for non-leaf functions and they supply the cogc driver with a stack so your more complicated function heirarchy will work. My driver intentionally only had a single level of function calls to avoid the need for a hub stack.

photomankc

What's a non-leaf function? Never heard that term before. My only other function call is i2cStretchHold() so I did end up creating a nested function there that I forgot about would that be the leaf or the non-leaf function?

ETA: Consulted the wise and powerful Google. So I converted all the other base functions to non-leaf by adding the function call to check for clock stretch so now the only leaf function is the i2cStretchHold. I have to check for clock stretching because my motor driver will do it now and then durring transfers. I did not want to make it a define because that would bloat out the code adding it in everywhere.

Never-the-less symtoms remain the same with the _NATIVE tags removed. -Os and it locks up when a call is made to the get the corrected acceleration values. -O0 operates as normal but is pushing 60KB.

David Betz

photomankc wrote: »

What's a non-leaf function? Never heard that term before.

A leaf function is a function that doesn't call any other functions. It is a leaf of the call tree. A non-leaf function does call other functions. I just disassembled your cogc driver and it is definitely generating stack operations. Here is a piece of the disassembly:

000000fc <_i2cSendByte>:
  fc:   84fc2004                        sub     40 <sp>, #4
 100:   083c1a10                        wrlong  34 <r13>, 40 <sp>
 104:   84fc2004                        sub     40 <sp>, #4
 108:   083c1c10                        wrlong  38 <r14>, 40 <sp>
 10c:   a0fc1a09                        mov     34 <r13>, #9
 110:   a0bc1c00                        mov     38 <r14>, 0 <r0>
 114:   5c7c0059                        jmp     #164 <.L9>

You'll have to either simplify the driver to get rid of these operations or you'll have to pass some stack space to your driver. I think this may explain a lot of the bizarre behavior you're seeing.

jazzed

Stack can be passed by defining the mailbox like this.

typedef volatile struct TB_Mailbox 
{
    uint32_t pins;      /* Pins to sample as touch buttons */
    uint32_t delay;     /* Delay between button samples */
    uint32_t btnStates; /* The most recent state of the buttons */
} Mailbox_t;


/*
 * This defines the structure which we'll pass to the C cog.
 */
struct par_s 
{
    unsigned int stack[16]; /* Provide a small stack for the COG */
    Mailbox_t mailbox;      /* Provide a mailbox to communicate to the COG */
};

Attached is a simple example where the mailbox is used that detects simple button + resistor pull-up states.