Summary: | ASTERISK-22130: Bridge API Enhancements - refactor Bridging API to hide protected functions and break up large file structure | ||
Reporter: | Matt Jordan (mjordan) | Labels: | Asterisk12 |
Date Opened: | 2013-07-20 15:59:52 | Date Closed: | 2013-07-24 23:10:01 |
Priority: | Major | Regression? | |
Status: | Closed/Complete | Components: | Core/Bridging |
Versions: | 12 | Frequency of Occurrence | |
Related Issues: | |||
Environment: | Attachments: | ||
Description: | We are dangerously close to once again creating a monolithic file that exposes implementation details to consumers that it shouldn't.
Fortunately, the Bridging API does have a lot of structure - it just needs to hide some the internals and be broken up into its various pieces. A significant amount of renaming should also be done. This includes: * Make (at a minimum) the following functions "protected" in a separate header file from {{bridging.h}}: ** ast_bridge_channel_state ** ast_bridge_channel_thread_state ** ast_bridge_action_type ** ast_bridge_methods and all function pointer type definitions ** ast_bridge_register ** ast_bridge_alloc ** ast_bridge_lock_both ** ast_bridge_base_init (protected) * Create factory functions that a create a bridge of a requested type. Ideally, we will have many types of sub-classed bridges. Construction of these bridges can be complex and require many flags. If someone wants they can go through the mechanics of customizing bridge creation; however, this is not ideal: {noformat} ast_mutex_lock(&bridgewait_lock); if (!holding_bridge) { holding_bridge = ast_bridge_base_new(AST_BRIDGE_CAPABILITY_HOLDING, AST_BRIDGE_FLAG_MERGE_INHIBIT_TO | AST_BRIDGE_FLAG_MERGE_INHIBIT_FROM | AST_BRIDGE_FLAG_SWAP_INHIBIT_FROM | AST_BRIDGE_FLAG_TRANSFER_PROHIBITED); } ast_mutex_unlock(&bridgewait_lock); {noformat} Ideally you should be able to request a subclass by its type, and get an instance of it. Things that produce bridges of a particular type would register a factory function that produces that bridge and associate it with that type. That will cut down on the header file inclusions and make this more flexible for further sub-classing. * Create a bridge_channel file and move all operations on bridge_channel to this file ** ast_bridge_channel_try_lock ** ast_bridge_channel_lock ** ast_bridge_channel_lock_bridge ** Rename ast_bridge_change_state_no_lock to indicate it is performed on a bridge_channel ** ast_bridge_change_state ** ast_bridge_update_linkedids - can probably just use bridge_channel ** ast_bridge_update_accountcode - can probably just use bridge_channel ** ast_bridge_channel_queue_frame ** ast_bridge_channel_queue_action_data ** ast_bridge_channel_write_action_data ** etc. * Perform a complete refactoring of names. The following nomenclature should be used: ** Public functions should use {{ast_[object]_[verb]_etc.}}. Objects are: *** {{bridge}} *** {{bridge_features}} *** {{bridge_channel}} *** Others will exist as well - but in general, we should not mix nomenclature. ** Protected functions should be prefixed with their namespace, but not {{ast}} ** Example: Rename ast_after_bridge_set_goto - to ast_bridge_set_after_goto ** Example: Rename ast_after_bridge_set_h to ast_bridge_set_after_h ** Continue with all of the ast_after... * Rename bridging_basic to bridge_feature or something similar. It is no longer a "basic" bridge * Decide what should be "bridging" versus "bridge" ** Potentially rename bridging_roles to something else - bridge_roles * Rename bridge_simple. It is not really "simple", it is a two party mixing bridge. | ||
Comments: |