Application of Mailbox

"data" is a stack-local variable in the ISR. It only lives while the ISR is active.

So why are you then sending the address of "data"?

isr_mbx_send(MSG_UART0, &data);

Note that isr_mbx_send() doesn't care much what it is sending. So you don't need to send pointers (addresses). You can send an integer value instead as long as the integer value isn't larger than the size of the pointer. And as long as the receiver understands that it doesn't get a pointer but gets a directly usable value.

"data" is a stack-local variable in the ISR. It only lives while the ISR is active.

So why are you then sending the address of "data"?

isr_mbx_send(MSG_UART0, &data);

Note that isr_mbx_send() doesn't care much what it is sending. So you don't need to send pointers (addresses). You can send an integer value instead as long as the integer value isn't larger than the size of the pointer. And as long as the receiver understands that it doesn't get a pointer but gets a directly usable value.

if i use the function isr_mbx_receive instead of os_mbx_wait, i get a Hardfault.

I also tried

                gData = LPC_UART0->RBR && READ_1BYTE;

                isr_evt_set(EVT_UART0, TID_IRQHandler);
                isr_mbx_send(MSG_UART0, (void*)gData);

gData is global. but still get the hardfault.

Seriously sorry.

mistake:

if i use the function isr_mbx_receive instead of os_mbx_wait, i get a Hardfault.

I get a HARDFAULT while using os_mbx_wait.
But dont get Hardfault, when i use isr_mbx_receive.

os_mbx_wait(&MSG_UART0, (void*) rx, 0xFFFF);

Have called os_mbx_init(), so your MSG_UART0 (why upper-case like it was a preprocessor symbol?) is valid for use with os_mbx_wait()?

And besides uppercase - MSG_UART0 does not sound like a mailbox. I would have expected the symbol name mbx_uart0.

Note that os_mbx_wait() takes an OS_ID as first parameter. The bad thing there is that it is just a void pointer so the compiler will not complain whatever pointer you may have. It is up to you to make sure it isn't a random pointer but a pointer to a initialized (!) mailbox object.

I still get a HARDFAULT while using os_mbx_wait.

_declare_box(MSGInbox, sizeof(uint32_t), 30);
os_mbx_declare(MSGUart0,2);


__task void TASK_IRQHandler(void)
{
        uint16_t rx_event=0;
        uint8_t *prx, data;

        os_mbx_init(MSGUart0, sizeof(MSGUart0));

        while(1)
        {
                os_evt_wait_or(CHECK_EVENT_FLAGS, INFINITE_TIME);
                rx_event = os_evt_get();

                switch(rx_event)
                {
                        case EVT_UART0:
                                os_mbx_wait(&MSGInbox, (void*) prx, 0xFFFF);
                                data = *prx;
                                _free_box(MSGInbox, prx);
                        break;
                }
        }
}

//_init_box(MSGInbox, sizeof(MSGInbox), sizeof(uint32_t)); has been done in Master_task




void UART0_IRQHandler(void)
{
        uint8_t interrupt_status, line_status;




        interrupt_status = 0x07 & (LPC_UART0->IIR >> 1);




        if(interrupt_status == UART_RDA)
        {
                gpData = _calloc_box(MSGInbox);




                *gpData = LPC_UART0->RBR & READ_1BYTE;




                isr_evt_set(EVT_UART0, TID_IRQHandler);
                isr_mbx_send(MSGUart0, (void *)gpData);
        }




        if(interrupt_status == UART_THRE)
        {
                line_status = LPC_UART0->LSR;
        }




}

The gpData is global. I also tried to do it by defining gpData as local variable.
but in both cases i get an HARDFAULT. I am unable to get what is wrong. Have tried many things but have proved to be a failure. Have referred to examples also and tried implementing, but still get an HARDFAULT. :|

Are you doing this on purpose?

Why do you mix a mailbox which is a method of communication, with the memory allocation method of fixed-size boxes?

If you send a mail to a mailbox, you obviously also should wait on that mailbox. Not involve any allocated box objects...

So you send to (the still very poorly named) MSGUart0 and you also wait on the (still very porly named) MSGUart0 mailbox.

os_mbx_declare(MSGUart0,2);

Wouldn't your code be much more obvious, if you don't name your mailbox "MSG"?

Wouldn't it be clearer if you post a mail to MailboxUart0, and you wait for mail from MailboxUart0?

Next thing: Why do you involve both a mail and an event for your ISR? Isn't it enough that it always send a mail, and you look at the mail to figure out if you received a character or something else happened?

And you have:

_declare_box(MSGInbox, sizeof(uint32_t), 30);

// init_box(MSGInbox, sizeof(MSGInbox), sizeof(uint32_t)); has been done in Master_task

But this isn't any inbox. This has nothing at all to do with any mail boxes. These are mails.

Another thing - you think you need to create a memory pool of 30 mails, when you then create a mailbox that can only keep track of two mails at a time:

os_mbx_declare(MSGUart0,2);

Where will all the other 28 mails be used?

Specific question: Are you just randomly testing things, without trying to understand the Keil documentation and trying to make a mental image of what the functions are doing, and how they interact? Does it seem to work well with that method of coding - giving you fast progress?

Working very hard to implement this concept. None of my colleagues have done this so far, hence a bit difficulty in understanding the concept alone. Anyways.... :)
Thanks for all your Patience. APPRECIATE :)

Why do you mix a mailbox which is a method of communication, with the memory allocation method of fixed-size boxes?
Mailbox is a method of transferring data;(& as per what i have understood) the data can be transferred only by allocating it some memory temperorily, right?

This is my new code and works well except this one extra warning
warning: #767-D: conversion from pointer to smaller variable

void UART0_IRQHandler(void)
{
        uint8_t interrupt_status, line_status, data;

        ...

        if(interrupt_status == UART_RDA)
        {

                data = LPC_UART0->RBR & READ_1BYTE;  //read the received byte

                if (isr_mbx_check (&MailBox) != 0)   //check if messages can be added
                        isr_mbx_send(&MailBox, (void *)data);
        }
        ...
}

__task void TASK_IRQHandler(void)
{
        uint8_t data;
        void *prx;




        while(1)
        {
                os_mbx_wait(&MailBox, &prx, 0xFFFF);    //wait till a new message is received
                data = (uint8_t)prx & READ_1BYTE;
        }
}

Have found out a thread on this post that explains how to deal with
warning: #767-D: conversion from pointer to smaller variable

Thanks a lots for the Guidance.

If multiple interrupts send mails, in such case, how do i identify the source of the mail(i.e. mailer). For this i had used events. Is this the right way or is there any other method to implement the same?

__task void TASK_IRQHandler(void)
{
        uint8_t data;
    uint16_t rx_event;
        void *prx;

        while(1)
    {
                os_evt_wait_or(CHECK_EVENT_FLAGS, INFINITE_TIME);
                rx_event = os_evt_get();

                switch(rx_event)
                {
                        case EVT_UART0:
                                os_mbx_wait(&MailBox, &prx, 0xFFFF);
                                data = (uint8_t)prx & READ_1BYTE;
                        break;

                        ...
                }
        }
}

"Mailbox is a method of transferring data;(& as per what i have understood) the data can be transferred only by allocating it some memory temperorily, right?"¨

Why do you ask this question, when you already have working code that sends the data using the pointer instead of allocating any memory for the pointer to point to? Haven't you already the answer to the question then? And I have also several times in previous posts covered the use of the pointer itself when the data you need to transfer is small enough that it fits in the pointer.

Next thing - programs normally send mails of a single type to a single mailbox. So one mailbox for data from that serial port. Another mailbox if receiving CAN message. So the receiving end knows what to do with the mail based on which mailbox it was picked up from.

Another alternative is to always tag each mail with a type. In the case of sending UART data in the pointer, you know that the data is 8 bit large. While the pointer is 32 bits. So 24 bits free for other information.

Like:

0x000000xx - received byte from UART0
0x000001xx - received byte from UART0, but with parity error
0x00000200 - framing error on UART0
...

Another solution here is to not use the mailbox at all, in case the receiving end wants to listen to lots of events. It works just as well to use a standard circular buffer for the received data. So the ISR inserts received bytes into the circular buffer and then sents an event signalling "At least one character received and available from UART0". The receiving end can then listen for many different event flags, and based on found flags decide what action to perform.

There are always many ways to skin a cat. The main goal is to not make everything too complex or too costly. Thinking about dynamic memory allocations inside the interrupt handler is a costly route.

By the way - why do you always want to use non-descriptive variable names? Now you have renamed your mailbox to just "MailBox", removing that reference to UART0.

And you have both an ISR and a task that both contains the name IRQHandler. With a name TASK_IRQHandler you are sending out a very mixed message if it is a task, or if it is an ISR.

Next thing - why do you do "& READ_1BYTE" both before sending your mail and after having received the mail? Never perform code actions without knowing exactly what the line does and exactly why you need it.