[asterisk-dev] [Code Review] LibPRI changes for Generic AOC

[rmudgett at digium.com]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/619/#review1861
-----------------------------------------------------------



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4017>

    I see Red spots.  For this file, you can simply trim all trailing whitespace from the file.



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4021>

    Change the function to this prototype and update the doxygen:
    
    void aoc_etsi_aoc_request(struct pri *ctrl, struct q931_call *call, const struct rose_msg_invoke *invoke)
    



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4018>

    Send a NotSubscribed ERROR back as a response.
    
    send_facility_error(ctrl, call, invoke->invoke_id, ROSE_ERROR_Gen_NotSubscribed);
    
    You also need to pass in the call pointer to the function to do this.



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4019>

    Send a NotImplemented ERROR back as a response.
    
    send_facility_error(ctrl, call, invoke->invoke_id, ROSE_ERROR_Gen_NotImplemented);



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4020>

    Send a NotAvailable ERROR back as a response.
    
    send_facility_error(ctrl, call, invoke->invoke_id, ROSE_ERROR_Gen_NotAvailable);



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4022>

    Not a parameter of aoc_subcmd_aoc_e_etsi_billing_id().



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4023>

    Value not coming from ROSE it is coming from libpri API.



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4024>

    Same as lines 688 and 689.



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4025>

    Delete the init line.  It is done inside the copy function.



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4026>

    Delete the init line.  It is done inside the copy function.



/team/rmudgett/aoc_event/pri_aoc.c
<https://reviewboard.asterisk.org/r/619/#comment4030>

    Add a TODO so the idea won't get lost:
    Add support to "timeout" when a specific message type is received.  In this case when CONNECT is received.
    
    "Timing out" when a specific message comes in would be a better solution than a simple timeout in this case.


- rmudgett


On 2010-04-20 12:51:37, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/619/
> -----------------------------------------------------------
> 
> (Updated 2010-04-20 12:51:37)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This review compliments the Asterisk changes in review https://reviewboard.asterisk.org/r/552/.  These are the LibPRI changes required to send ETSI AOC messages generated by Asterisk.
> 
> 
> Diffs
> -----
> 
>   /team/rmudgett/aoc_event/pri_facility.h 1641 
>   /team/rmudgett/aoc_event/pri_facility.c 1641 
>   /team/rmudgett/aoc_event/pri_internal.h 1641 
>   /team/rmudgett/aoc_event/q931.c 1641 
>   /team/rmudgett/aoc_event/libpri.h 1641 
>   /team/rmudgett/aoc_event/pri_aoc.c 1641 
> 
> Diff: https://reviewboard.asterisk.org/r/619/diff
> 
> 
> Testing
> -------
> 
> These routines have been tested back to back with Asterisk, but I am going to need some community help to test interoperability.
> 
> To test this code, check out both my Asterisk and Libpri changes for AOC
> 
> Asterisk Changes: svn/asterisk/team/dvossel/generic_aoc
> LibPRI Changes: svn/libpri/team/dvossel/aoc_send 
> 
> 
> Thanks,
> 
> David
> 
>