NordicPlayground / nRF51-ble-bcast-mesh

Other
323 stars 121 forks source link

BLE_Gateway with serial_handler_uart #62

Closed cpignol closed 8 years ago

cpignol commented 8 years ago

Hi, I'm working with the code from Branch sdk-8-support and would like to replace the spi protocol by the uart protocol. So I replaced the serial_hanlder_spi.c by serial_handler_uart.c (from the src directory). define RBC_MESH_SERIAL in main. Compile OK but the commands sent to the serial are never processed. It seems that serial_command_handler is never called. What am I doing wrong? Thanks,

rifatmahmud commented 8 years ago

Double check the baud rate and other configurations. Also, if possible, put the main file on the gist and show it to us.

daviddedwin commented 8 years ago

Can you take a look at the dfu_alpha branch that has UART support, https://github.com/NordicSemiconductor/nRF51-ble-bcast-mesh/tree/dfu_alpha/nRF51/bootloader

cpignol commented 8 years ago

Thanks for the info and I made good progress. I used the same asynchronous method with SWI1 to call do_transmit() and mesh_aci_command_check() in serial_handler_uart.c. I used only one interrupt handler because SWI2 is used by the SoftDevice. void SWI1_IRQHandler(void) { if (interruptFlag ==1 ) do_transmit(); if (interruptFlag ==2 ) mesh_aci_command_check(); } I can now send the serial command (opcode 0x70) to initialize the mesh and get the command response. I can also do a value get (opcode 0x7A). I can change the value of one handle (opcode 0x71). It stops working if I try to do a second command to change an handle value (0x71). The program is in error loop general handler error. Thanks and happy new year.

trond-snekvik commented 8 years ago

Hi, are you able to get a stack trace on the error?

cpignol commented 8 years ago

Hi, Serial communication: Looking at the command set handle value opcode 0x71 an comparing with the value of the characteristic on my phone, I think the value of the data length should be app_evt.data_len = serial_cmd->length - 3; and serial_cmd->length - 3 ); because 1byte for the opcode and 2 bytes for the handle. (Excel doc) I can change the value from my phone and turn ON/OFF the LEDs.

The only thing that doesn't work for the serial command set is changing the value of the LED. The value of evt->data[0] in rbc_mesh_event_handler(rbc_mesh_event_t* evt) is not correct. In the memory I have the value 0xBE instead of 0x01 or 0x00. The 32 bit value is 0xCAFEBABE, With the debugger I was able to check the bc_mesh_event_push( p_event) and the data value is correct. Something happen when rbc_mesh_event_get(&evt) is executed. This is perfectly repetitive. Is 0xCAFEBABE has a special meaning for the SD? I have tried many things without success. My hardware is PCA10028 nRF51822_xxAB

The command get handle value opcode 0x7A I modify the test of _length if(_length < p_adv_data->adv_data_length - MESH_PACKET_ADV_OVERHEAD)
in version_handler.c. I think it should be the opposite. The value length was not correct before the test and the test fix it to the correct value. I don't know yet why length was not the right value? Doing that the command 0x7A works fine. With this command I can also check command set handle value and it's correct. Thanks

trond-snekvik commented 8 years ago

Looks like we have a few framework bugs on our hands!

For 0x71: I agree, it should be - 3 (or optimally not a magic number), this is a slip from the transition from 1-byte handles to 2-byte handles. This goes both for the call to rbc_mesh_value_set() and rbc_mesh_event_push().

For 0x7A: You're correct, this is another bug.

The CAFEBABE-bug is a bit nastier. In short, we're pointing to the serial-command buffer directly - a buffer which is stack allocated at line 487, and which will be invalid by the time the event is processed. Here's a temporary work-around, replace the VALUE_SET handling in mesh_aci.c::serial_command_handler to:

    case SERIAL_CMD_OPCODE_VALUE_SET:
    {
        serial_evt.opcode = SERIAL_EVT_OPCODE_CMD_RSP;
        serial_evt.params.cmd_rsp.command_opcode = serial_cmd->opcode;
        serial_evt.length = 3;

        mesh_packet_t* p_packet;
        if (mesh_packet_acquire(&p_packet))
        {
            if (serial_cmd->length > sizeof(serial_cmd_params_value_set_t) + 1)
            {
                serial_evt.params.cmd_rsp.status = ACI_STATUS_ERROR_INVALID_LENGTH;
                error_code = NRF_ERROR_INVALID_LENGTH;
            }
            else
            {
                error_code = rbc_mesh_value_set(   serial_cmd->params.value_set.handle,
                                                            serial_cmd->params.value_set.value,
                                                            serial_cmd->length - 3);

                serial_evt.params.cmd_rsp.status = error_code_translate(error_code);
            }

            /* notify application */
            if (error_code == NRF_SUCCESS)
            {
                memcpy(p_packet->payload, serial_cmd->params.value_set.value, serial_cmd->length - 3);
                memset(&app_evt, 0, sizeof(app_evt));
                app_evt.event_type = RBC_MESH_EVENT_TYPE_UPDATE_VAL;
                app_evt.data = p_packet->payload;
                app_evt.data_len = serial_cmd->length - 3;
                app_evt.value_handle = serial_cmd->params.value_set.handle;

                error_code = rbc_mesh_event_push(&app_evt);
                if (error_code != NRF_SUCCESS)
                {
                    mesh_packet_ref_count_dec(p_packet);
                }

                serial_evt.params.cmd_rsp.status = error_code_translate(error_code);
            }
        }
        else
        {
            serial_evt.params.cmd_rsp.status = error_code_translate(NRF_ERROR_BUSY);
        }

        serial_handler_event_send(&serial_evt);
        break;
    }

This uses the packet-buffer in a similar fashion to the transport-layer for radio RX. Remember to include "mesh_packet.h" in mesh_aci.c for this.

Does it fix your issue?

cpignol commented 8 years ago

Thanks for your support. The modifications of the SERIAL_CMD_OPCODE_VALUE_SET doesn't fix the CAFEBABE bug. I am using the code from the branch sdk-8-support. I am not sure which code is referenced by the line 487. What is line number in the original mesh_ai.c file?

trond-snekvik commented 8 years ago

Oh, sorry, my bad. I've edited the original post, as I forgot actually feeding the statically allocated data to the event-queue. Try it now!

cpignol commented 8 years ago

Yep, make sense... Thank you

cpignol commented 8 years ago

Hi,

Testing the new code, it works until we run out of p_packet. Need to add a call to mesh_packet_ref_count_dec(p_packet) after rbc_mesh_packet_release(evt.data); in main.c I don't a have an elegant solution. Just store in a global variable the address p_packet to be able to do the call in main.c Thanks,

trond-snekvik commented 8 years ago

Try calling mesh_packet_ref_count_dec(p_packet) after event_push regardless of errors in my snippet. If successful, the event_push-function should have created it's own reference, and you could safely remove the one created earlier in the snippet. I think I was a little too quick while proposing a quick fix. Sorry about that.

cpignol commented 8 years ago

No problem it was easy to figure out where the problem was. Thanks for the support.