Uniform plugin API design and plugin management

[djordjeglbvc]

Summary Several places across Access SDK rely on user created plugins (eg. PEP, PIP, data, storage, hsm). There is a need to create unified way for handling and managing plugins.

I will put design propositions in comments, to open discussions and get feedback.

[djordjeglbvc]

Universal plugin API proposal

Plugin is defined as a set of callbacks which are assigned to a plugin structure object. Plugin's functionalities are accessed through plugin API, by passing plugin object to it.

Plugin API

Plugin API functions which are universal for all plugins, and corresponding structure would look something like this:

typedef int (*plugin_cb)(plugin_t* plugin, void* user_data);
typedef struct {
  plugin_cb destroy;
  plugin_cb *callbacks;
  size_t callbacks_num;
  void *plugin_specific_data;
} plugin_t;

int plugin_init(plugin_t *plugin, void (*initializer)(plugin_t*));
int plugin_destroy(plugin_t *plugin);
int plugin_call(plugin_t *plugin, size_t plugin_idx, void* user_data);

Initializer function is implemented by user, it is used to fill plugin_t structure with callbacks and plugin related data. Destroy callback is universal, so it is declared as a separate field in structure. Depending on module using plugins, number and function of callback would be variable. For instance, here is just as an illustration for resolver plugin (note: will be renamed to pep_plugin, callback functions will be changed...), we have following functions: init_ds_interface, start_ds_interface, stop_ds_interface and term_ds_interface_cb. term_ds_interface_cb will be assigned to destroy callback, so that leaves us with 3 more callbacks. For these, we would define 3 functions, also we would define 3 index enumerations like this:

// PEP common header
typedef enum {
  INIT_DS_INTERFACE = 0, 
  START_DS_INTERFACE,
  STOP_DS_INTERFACE
}

// plugin implementation header
int initializer(plugin_t*);

// plugin implementation c
static int deinit_ds_interface(plugin_t *plugin)
{
  free(plugin->callbacks);
  free(plugin->plugin_specific_data);
}
int initializer(plugin_t* plugin)
{
  plugin->plugin_specific_data = malloc(PLUGIN_SPECIFIC_DATA_SIZE);
  plugin->callbacks = malloc(sizeof(void*) * 3); // 3 callbacks
  plugin->callbacks[INIT_DS_INTERFACE] = init_ds_interface; // static function
  plugin->callbacks[START_DS_INTERFACE] = start_ds_interface; // static function
  plugin->callbacks[STOP_DS_INTERFACE] = stop_ds_interface; // static function
  plugin->destroy = deinit_ds_interface; // static function
}

This could be use at all places where we have some kind of plugins. @strahinjagolic please take a look at this, would it be appliable to your tasks which include plugin interfacing?

Plugin management

I would propose a simple API for plugin management.

typedef struct {
  plugin_t* plugins;
  size_t plugins_max_num;
  size_t plugins_num;
} plugin_db_t;

int plugin_manger_init(plugin_db_t*, size_t plugins_max_num);
int plugin_manager_register(plugin_db_t*, plugin_t*);
int plugin_manager_get(plugin_db_t*, size_t idx, plugin_t*);

Each module which uses plugins would instantiate its own plugin_db_t, add plugins to it using _register api, and access it using _get api. Plugins array could be used as dynamically allocated array, or statically if we are on some bare-metal microcontroller system.

[bernardoaraujor]

@djordjeglbvc thanks for your explanation. I need some more time to deeply understand them and provide some useful feedback but I like the direction this is going.

[bernardoaraujor]

@djordjeglbvc @strahinjagolic I'm reflecting about:

typedef enum {
  INIT_DS_INTERFACE = 0, 
  START_DS_INTERFACE,
  STOP_DS_INTERFACE
}

wouldn't it make more sense to place this in the plugin implementation header, instead of PEP common header? The reason for my question is that not all PEP plugins will necessarily have data sharing actions.

Take for example my draft implementation of pep_plugin_relay.

All data sharing functions were left out empty, because they don't make much sense in the context of this specific plugin. But having the proposed enum in the PEP plugin common header forces us to still implement them.

But when it comes to something like GPS or CAN bus, they do make sense.

So I propose having such enum written on the plugin implementation header instead. For example on pep_plugin_relay.h we would have:

typedef enum {
  RELAY_ON = 0, 
  RELAY_OFF,
  RELAY_TOGGLE,
  RELAY_PULSE
}

While on pep_plugin_can_01.h we would have:

typedef enum {
  INIT_DS_INTERFACE = 0, 
  START_DS_INTERFACE,
  STOP_DS_INTERFACE,
  OPEN_DOOR,
  CLOSE_DOOR,
  OPEN_TRUNK,
  START_ENGINE
}

Let me know what you guys think.

[bernardoaraujor]

@djordjeglbvc @strahinjagolic In regards to Plugin Management, I would propose the following workflow:

The initial Reference Implementation will have the following plugins: PEP:

• Relay
• Wallet (for token collection)

PIP:

• GPIO (for reading pin state)
• Wallet (tx verification)

on main.c, developer:

1. declares desired plugins
2. initializes the desired plugins
3. initializes plugin db
4. registers desired plugins to db
5. initializes Access Actor with plugin db

   
   static access_ctx_t *access_context;
   static plugin_db_t *plugin_db;
   ...

// 1 plugin_t pep_plugin_relay; plugin_t pep_plugin_wallet; plugin_t pip_plugin_gpio; plugin_t pip_plugin_wallet;

// 2 plugin_init(pep_plugin_relay, pep_plugin_relay_initializer); plugin_init(pep_plugin_wallet, pep_plugin_wallet_initializer); plugin_init(pip_plugin_gpio, pip_plugin_gpio_initializer); plugin_init(pip_plugin_wallet, pip_plugin_wallet_initializer);

// 3 plugin_manger_init(plugin_db, 4);

// 4 plugin_manager_register(plugin_db, pep_plugin_relay); plugin_manager_register(plugin_db, pep_plugin_wallet); plugin_manager_register(plugin_db, pip_plugin_gpio); plugin_manager_register(plugin_db, pip_plugin_wallet);

// 5 access_init(access_context, plugin_db);

[bernardoaraujor]

we might need an extra field on plugin_t to differentiate between plugin categories.

typedef enum {
  PAP_PLUGIN,
  PEP_PLUGIN,
  PIP_PLUGIN
} plugin_category_t;

typedef struct {
...
  plugin_category_t category;
} plugin_t;

then on access.c, the access_init() function loops over *plugin_db and calls the appropriate register function:

• pap_register_callback()
• pep_register_callback()
• pip_register_callback()

where switch (plugin->category) is used to decide which register function to call.

[djordjeglbvc]

@djordjeglbvc @strahinjagolic I'm reflecting about:

typedef enum {
  INIT_DS_INTERFACE = 0, 
  START_DS_INTERFACE,
  STOP_DS_INTERFACE
}

wouldn't it make more sense to place this in the plugin implementation header, instead of PEP common header? The reason for my question is that not all PEP plugins will necessarily have data sharing actions.

These functions names are for illustration only, I will dive into specifics for each group together with @strahinjagolic. It makes sense for each type of plugin to have universal interface, if we don't use some functionality, we will simply have empty callback, but for each group of plugins we should have same interface.

Take for example my draft implementation of pep_plugin_relay.

All data sharing functions were left out empty, because they don't make much sense in the context of this specific plugin. But having the proposed enum in the PEP plugin common header forces us to still implement them.

It doesn't force us to implement them, it only defines unified interface for one plugin group. If we don't need some functionality, we can have empty or NULL callbacks. And action set shouldn't be part of plugin api, it is something internal to a specific plugin. In any way, these are implementation specifics, and I want first to make things clear with api design.

But when it comes to something like GPS or CAN bus, they do make sense.

So I propose having such enum written on the plugin implementation header instead. For example on pep_plugin_relay.h we would have:

typedef enum {
  RELAY_ON = 0, 
  RELAY_OFF,
  RELAY_TOGGLE,
  RELAY_PULSE
}

While on pep_plugin_can_01.h we would have:

typedef enum {
  INIT_DS_INTERFACE = 0, 
  START_DS_INTERFACE,
  STOP_DS_INTERFACE,
  OPEN_DOOR,
  CLOSE_DOOR,
  OPEN_TRUNK,
  START_ENGINE
}

Let me know what you guys think. Again, INIT_DS_INTERFACE, etc, aren't real functions which will be in plugin interface, it is here only for illustration. Functions will most likely be something like PROCESS_INPUT, START_PLUGIN, STOP_PLUGIN (not really, but something generic like that), which will be universal to all plugins (from the same group). Access SDK doesn't know what OPEN_DOOR, etc means, this is something that will be internal to specific plugin, and only visible from within the plugin (static functions). Perhaps there will be some inter-plugin communication in regards to this, but these things shouldn't be visible to plugin consumer (eg. access sdk).

I will discuss about specifics of plugin interfaces for each of the groups (pep, pip, etc) with @strahinjagolic, to be in sync with what is the best for modules he is more familiar with.

we might need an extra field on plugin_t to differentiate between plugin categories.

typedef enum {
  PAP_PLUGIN,
  PEP_PLUGIN,
  PIP_PLUGIN
} plugin_category_t;

typedef struct {
...
  plugin_category_t category;
} plugin_t;

No need, as each of the modules (PEP, PIP, PAP, etc) will know what plugin callbacks to expect. Adding this enum would make plugin interface less generic, for each new type we would need to extend enum, and it wouldn't make our lives easier.

Access api would have functions like

access_register_pap_plugin(plugin_t*);
access_register_pep_plugin(plugin_t*);
access_register_pip_plugin(plugin_t*);

Which would all in turn call plugin_manager_register(plugin_db_t*, plugin_t*);, just with different plugin_db parameter for each plugin type.

User which implements plugin would need to take care about which plugin type requires which callbacks, implement them accordingly and add it to the right db using right access_register_*_plugin() api call. Type field would also need to be filled in by the user when creating/instancing plugin, so it doesn't really add another level of safety.

[djordjeglbvc]

on main.c, developer:

1. declares desired plugins

2. initializes the desired plugins

3. initializes plugin db

4. registers desired plugins to db

5. initializes Access Actor with plugin db

@bernardoaraujor developer woudn't initialize plugin db, plugin db instance is initialized for each of the plugin consumer modules (pap, pep, pip) inside of access or inside of individual pp modules, I will see where it makes more sense from implementations point of view when it comes to that. But this should not be visible to the user. User just creates plugin, and registers it through access api (eg `access_register_pep_plugin(plugin_t);`

[strahinjagolic]

@djordjeglbvc @bernardoaraujor After reading all of this, I must say that I'm not sure if we can pull this off like this. Main reason is that every plugin is specific and I can't say, at this moment, is it possible to have unified plugin API, although, it seems like a good idea. We can try going in this direction, but if we hit the wall, we will must make some compromise.

@djordjeglbvc What I don't like is the name plugin_db_t. Please find some more descriptive name, because "db" can mean a lot of things. Also, you sad

term_ds_interface_cb will be assigned to destroy

As I understood, destroy should destroy plugin and term data sharing is different thing, so we should leave this callback. Correct me if I'm wrong.

[djordjeglbvc]

@strahinjagolic I don't think that it is complicated, there are really only a few callbacks per plugin type which we would need just to move into the new plugin_t structure, and move their implementation and instantiation to main, which we would have to do any way.

Regarding modules in which we haven't yet added any plugin support, situation should be even easier when we decide on plugin api. And if we don't get the correct callback setup for certain plugin type, we can easily add or remove them, as interface is generic.

Don't go too deeply into term_ds_interface_cb, I left it there just for illustration purposes, we will discuss about actual callback names and functions after we agree on plugin api principles and see how it all fits in existing modules.

Rename plugin_db_t to plugin_database_t? I mean all these names are currently open for suggestions, but it is important to come to some conclusions about universal plugin api and management principles.

[strahinjagolic]

@djordjeglbvc Let me check if I understand: The callbacks field of the plugin_t is a pointer to an array of callbacks and we can choose specific one by indexing it via fields from enum. Is this correct?

Rename plugin_db_t to plugin_database_t?

Bether, yes.

[djordjeglbvc]

@djordjeglbvc Let me check if I understand: The callbacks field of the plugin_t is a pointer to an array of callbacks and we can choose specific one by indexing it via fields from enum. Is this correct?

@strahinjagolic Yes, that was my idea. Enum would be defined in header linked to plugin consumer module (pep, pip, etc). It might be for instance pep.h, or perhaps add new header which is to be used as sort of plugin implementation guide, and call it pep_plugin.h or something like that. These are just ideas, but I think this would make things simple, tidy, and easily manageable by a small team like ours.

[bernardoaraujor]

what if instead of plugin_database_t we do plugin_manager_t? since the function calls are already called plugin_manager_*

[djordjeglbvc]

@bernardoaraujor yes, we can name it plugin_manager_t to make it consistent with api calls.

[bernardoaraujor]

@djordjeglbvc

It doesn't force us to implement them, it only defines unified interface for one plugin group. If we don't need some functionality, we can have empty or NULL callbacks.

But that's tricky.

a pep_plugin_wallet will have radically different behaviour than pep_plugin_relay or pep_plugin_can

don't be fooled by the fact that all of them still belong to the same category PEP, because:

• datasharing actions only make sense for pep_plugin_can
• send transaction only makes sense for pep_plugin_wallet
• toggle relay only makes sense for pep_plugin_wallet

if we have a unified enum, all these actions are forced upon all plugin implementations, even if they all belong to PEP.

right @strahinjagolic ?

[djordjeglbvc]

@bernardoaraujor why wouldn't we treat datasharing, send transaction and toggle relay as same thing? Logic here is that all of these actions send data out.

These callbacks will all be called from within the same loop which iterates through all registered plugins.

[bernardoaraujor]

@bernardoaraujor why wouldn't we treat datasharing, send transaction and toggle relay as same thing? Logic here is that all of these actions send data out.

I think that's also not that simple. For example, on relay we have four different actions:

• on
• off
• toggle
• pulse

one could argue all those actions are output. In fact, everything a PEP plugin does is categorized as output, while PIP is input. But there can be different kinds of output and input for each different plugin.

[djordjeglbvc]

We should look at plugin interface from (for instance) pep's point of view - what is the purpose of that module?

It checks if action is in sync with a valid policy, if it checks out - it passes action to a callbacks of registered plugins (lets call it action_cb). Action_cb of each plugin determines if it should process that action (user has responsibility to implement these checks), if it does, it executes that action. That action could be share data, toggle relay, send transaction or something completely different.

[djordjeglbvc]

@bernardoaraujor why wouldn't we treat datasharing, send transaction and toggle relay as same thing? Logic here is that all of these actions send data out.

I think that's also not that simple. For example, on relay we have four different actions:

* on

* off

* toggle

* pulse

one could argue all those actions are output. In fact, everything a PEP plugin does is categorized as output, while PIP is input. But there can be different kinds of output and input for each different plugin.

Plugin's callback should have to know how to process these actions, which are passed as an argument to a callback. void* user_data, for instance.

[djordjeglbvc]

Just to stress it once more - pep shouldn't need to know what toggle relay, share data or send transaction is. Thus plugin api doesn't need to know anything about it. PEP only knows that some data should be sent through a plugin, and simplest way in my opinion is to try and send the action through all of the registered plugins, but it will be executed only by the plugin which recognizes the action. PEP doesn't know semantics of the action, only if policy allows for it to be executed. Plugin implementation takes care of semantics.

[bernardoaraujor]

Plugin's callback should have to know how to process these actions, which are passed as an argument to a callback. void* user_data, for instance.

but then the discussion is no longer about action enum?

Maybe an alternative approach could be to use generic action numbers, so instead of

typedef enum {
  RELAY_ON = 0, 
  RELAY_OFF,
  RELAY_TOGGLE,
  RELAY_PULSE
}

or

typedef enum {
  INIT_DS_INTERFACE = 0, 
  START_DS_INTERFACE,
  STOP_DS_INTERFACE
}

or

typedef enum {
  SEND_TRANSACTION = 0
}

we would only have:

size_t n_actions;

just to avoid hitting empty memory slots. And then, for each PEP plugin implementation there would be a mapping of each position to a specific action.

So in wallet:

int initializer(plugin_t* plugin)
{
  plugin->plugin_specific_data = malloc(PLUGIN_SPECIFIC_DATA_SIZE);
  plugin->callbacks = malloc(sizeof(void*) * 1); // 1 callbacks
  plugin->callbacks[0] = send_transaction; // static function
  plugin->destroy = destroy_wallet; // static function
}

in relay:

int initializer(plugin_t* plugin)
{
  plugin->plugin_specific_data = malloc(PLUGIN_SPECIFIC_DATA_SIZE);
  plugin->callbacks = malloc(sizeof(void*) * 1); // 4 callbacks
  plugin->callbacks[0] = relay_on; // static function
  plugin->callbacks[1] = relay_off; // static function
  plugin->callbacks[2] = relay_toggle; // static function
  plugin->callbacks[3] = relay_pulse; // static function
  plugin->destroy = destroy_relay; // static function
}

and in can:

int initializer(plugin_t* plugin)
{
  plugin->plugin_specific_data = malloc(PLUGIN_SPECIFIC_DATA_SIZE);
  plugin->callbacks = malloc(sizeof(void*) * 3); // 3 callbacks
  plugin->callbacks[0] = init_ds_interface; // static function
  plugin->callbacks[1] = start_ds_interface; // static function
  plugin->callbacks[2] = stop_ds_interface; // static function
  plugin->destroy = deinit_ds_interface; // static function
}

If I'm not mistaken, this is also aligned with the approach that @vlad-ns has proposed for Mobile Client.

I just don't want to impose actions such as *_ds_interface to all plugins if not all of them will need them.

[bernardoaraujor]

Just to stress it once more - pep shouldn't need to know what toggle relay, share data or send transaction is.

I think the generic approach above follows this philosophy. But it makes better use of memory, avoiding empty action slots.

[djordjeglbvc]

@bernardoaraujor plugin interface doesn't include callbacks for actions. Plugin callbacks are functions which would be directly called by pep module, for instance. All pep plugins would have 2 or 3 generic callbacks (maybe even less, we will see how many we really need), and no custom callbacks.

Actions are handled internally inside of plugin implementation, and are not visible outside of plugin implementation. They will most of the time be just static functions called from within some if else construct, but that doesn't matter as they don't need to be visible outside of the plugin. At least not visible by access sdk.

And there will be no empty memory slots.

[djordjeglbvc]

I may have introduced some confusion with example I have used, sorry for that.

[djordjeglbvc]

Initializer would look something like this for all pep plugin implementations

int initializer(plugin_t* plugin)
{
  plugin->plugin_specific_data = malloc(PLUGIN_SPECIFIC_DATA_SIZE);
  plugin->callbacks = malloc(sizeof(void*) * 1); // 1 callbacks
  plugin->callbacks[0] = execute_action_cb; // static function
  plugin->destroy = destroy_cb; // static function
}

edit: of course we wouldn't use 1, but something like #define PEP_CALLBACKS_COUNT 1 which would be defined in pep_plugin.h for instance, which would be used as a sort of a plugin implementation guide.

[bernardoaraujor]

then what's the point of having callbacks as an array?

[djordjeglbvc]

then what's the point of having callbacks as an array?

Making it generic, some other parts of the system would need to have more than one callback per plugin, like some stop, start, restart callbacks.

[djordjeglbvc]

It would perhaps turn out that we need more than 1 callback for pep when we start working on it, I gave this only as an illustration, again.

I put only one mandatory callback, destroy, because we will always need a callback to clean up allocated memory at the end of the run-time.

[bernardoaraujor]

@djordjeglbvc so taking a step back and making a summary of your conceptual approach (to make sure I understand correctly):

for PEP, you propose to have callbacks for the following conceptual entities:

• action, where the specific action identifier is passed as void* options arg
• init_ds_interface, optional
• start_ds_interface, optional
• stop_ds_interface, optional
• destroy, where this is not defined in the array as it's mandatory for all plugins

if I understand correctly, I think it makes sense. let's see if @strahinjagolic agrees.

also, looking from this angle, it might make sense to use the enum you proposed. maybe do it like this?

typedef enum {
  ACTION = 0,
  INIT_DS_INTERFACE, 
  START_DS_INTERFACE,
  STOP_DS_INTERFACE
}

[djordjeglbvc]

@bernardoaraujor something like that, although we probably wouldn't call additional callbacks init_ds_interface, etc as these doesn't make sense in this context. But other than that, yes.

[strahinjagolic]

Well, this is confusing!

First, keep in mind that not only the PEP module has plugins. Think of the PAP plugin, for example. It has 6 different callbacks, each of them with different parameters. Does this mean that we will need to have structure for each of them, which we will send as void* user_data?

Second, regarding data sharing, keep in mind that start/stop data sharing are actions like any other, so maybe we shouldn't treat it as a special case. But this is another topic, I'm feeling like we have diverged from original topic with this one.

Third, I can't see the point of the enum if we are going to shoot all callbacks, if I understood correctly. And, also, what about the PAP or PIP, if we have callbacks and enums how will that look like there? I think that @bernardoaraujor mentioned that we shouldn't have enum describing plugin related operations in P*P headers. I fully agree with this one. But in that case, I'm not sure where to keep it (enum), do we need it at all and how are we going to use it?

I'm feeling that proposed solution might be a good idea, but we still need to do some re-defining. Or, maybe, I completely failed to understand it, which is not so implausible.

[bernardoaraujor]

Think of the PAP plugin, for example. It has 6 different callbacks, each of them with different parameters. Does this mean that we will need to have structure for each of them, which we will send as void* user_data?

As I understood, the answer is no.

All plugins (PAP, PEP and PIP) will be instances of plugin_t. They will each have different initializer Implementations, where the callback array is constructed on each instance. The enum would come in here (potentially different enum for each PxP), but that's just a minor detail.

I recognize the void *user_data vs enum made things unnecessarily confusing, so let's leave them aside for the moment.

The point is to have the same plugin_t for all plugins, regardless of which module they belong to. And then differentiate functionality on initializer and callback array.

[djordjeglbvc]

Well, this is confusing!

First, keep in mind that not only the PEP module has plugins. Think of the PAP plugin, for example. It has 6 different callbacks, each of them with different parameters. Does this mean that we will need to have structure for each of them, which we will send as void* user_data?

Some callbacks would need user data, some maybe not. It doesn't have to be a structure, it can be any kind of data. In any way you would need different parameters for different callbacks.

Second, regarding data sharing, keep in mind that start/stop data sharing are actions like any other, so maybe we shouldn't treat it as a special case. But this is another topic, I'm feeling like we have diverged from original topic with this one.

Start/stop data sharing actions will not be handled by separate plugin callbacks, they will be handled like any other action, inside plugin implementation, inside action_cb, for instance. I've used start/stop only as an example, not related to data sharing. I could use init/reinit/deinit/duplicate/fetch.

Third, I can't see the point of the enum if we are going to shoot all callbacks, if I understood correctly. And, also, what about the PAP or PIP, if we have callbacks and enums how will that look like there? I think that @bernardoaraujor mentioned that we shouldn't have enum describing plugin related operations in P*P headers. I fully agree with this one. But in that case, I'm not sure where to keep it (enum), do we need it at all and how are we going to use it?

I'm feeling that proposed solution might be a good idea, but we still need to do some re-defining. Or, maybe, I completely failed to understand it, which is not so implausible.

We will not shoot all callbacks, we will call one particular callback from all of the registered plugins. So we are iterating through each registered plugin, and calling action_cb, for instance.

I am currently working on example on my branch, redoing resolver to use this plugin system. Have in mind that this is only for illustration, I know resolver will be renamed to pep_plugin, but only to illustrate how I have imagined for plugin system to be used.

[strahinjagolic]

We will not shoot all callbacks, we will call one particular callback from all of the registered plugins. So we are iterating through each registered plugin, and calling action_cb, for instance.

This is OK for PEP. But how will it look like for e.g. PAP case? There, we only have one plugin registered (Storage plugin, or pap_plugin, if you like) and this one plugin has 6 callbacks. Do you think that this plugin will have structure containing array of callbacks and should we use enum for this case in order to call particular cb?

[bernardoaraujor]

There, we only have one plugin registered (Storage plugin, or pap_plugin, if you like)

is that entirely true? because as I understood, there could be different approaches to storing policies (e.g.: unix file vs bare metal non volatile mem vs secure element). That would already qualify PAP as having more than one possible plugin.

My view is that pap_plugin is an interface for different Implementations, same way as pep_plugin and relay, can, wallet etc.

Do you think that this plugin will have structure containing array of callbacks and should we use enum for this case in order to call particular cb?

As I understood @djordjeglbvc 's approach, in each categoey (PAP vs PEP vs PIP) every callback with fixed purpose across all plugins gets a slot in the enum.

If in the same plugin, different callbacks have similar functionality, they get the same enum slot and then is differentiated with void *user_data (perhaps *options is a better name?)

In the case of PAP, its enum would probably have 6 callbacks.

[strahinjagolic]

is that entirely true? because as I understood, there could be different approaches to storing policies (e.g.: unix file vs bare metal non volatile mem). That would already qualify PAP as having more than one possible plugin

I think we have misunderstanding regarding terminology. I've noticed that you call "demos" plugins (plugin_wallet, plagin_can, ...), but we say plugin when we talk about resolver module, e.g. Same way storage plugin is only one, but it can include multiple interfaces for physical storage. We should agree on terminology ASAP. In theory, PAP can have multiple plugins registered, but I was just making an example.

As I understood @djordjeglbvc 's approach, in each categoey (PAP vs PEP vs PIP) every callback with fixed purpose across all plugins gets a slot in the enum.

I don's see possibility for callbacks with same purpose across all plugins. Can you give me an example, please?

[djordjeglbvc]

We will not shoot all callbacks, we will call one particular callback from all of the registered plugins. So we are iterating through each registered plugin, and calling action_cb, for instance.

This is OK for PEP. But how will it look like for e.g. PAP case? There, we only have one plugin registered (Storage plugin, or pap_plugin, if you like) and this one plugin has 6 callbacks. Do you think that this plugin will have structure containing array of callbacks and should we use enum for this case in order to call particular cb?

Enums are there to help plugin consumer to identify which function it is calling. For instance, if we have callback called action_cb, for it we have enum field ACTION_CB=0.

From pep's point of view, it would be called like this:

for (int i = 0; i <plugin_manager->plugins_num; i++)
{
  plugin_t *plugin;
  void* user_data = something...;
  pluginmanager_get(plugin_manager, i, &plugin);
  plugin_call(plugin, ACTION_CB, user_data);
}

So plugins called by plugin_call are indexed using enum, and they are assigned to callback array.

There is no problem if we have only one registered plugin, it will call that particular callback from only one plugin instance. If there are 6 different callbacks, they will be called from 6 different places in PAP module, but only on that one plugin instance. If we decide that PAP, for instance, will only ever need one plugin instance at a time, we can skip plugin management for PAP, we will only assign one plugin to PAP context, and call callbacks using that plugin instance.

[djordjeglbvc]

I don's see possibility for callbacks with same purpose across all plugins. Can you give me an example, please?

All plugins from same group will have same callbacks, that is the purpose of plugins. You can just replace one with another.

PEP and PAP plugins will not have same callbacks, for instance, but all PAP plugins will have same set of callbacks.

[strahinjagolic]

@djordjeglbvc

So, in PAP, it will be something like:

void pap_get() {
    plugin_t *plugin;
    void* user_data = something...;
    pluginmanager_get(plugin_manager, i, &plugin);
    plugin_call(plugin, GET_CB, user_data);
}

[djordjeglbvc]

@strahinjagolic if we decide we need only one plugin for PAP, pluginmanager_get step isn't even necessary.

[strahinjagolic]

OK, I understand now. I think we can do it this way.Thanks.

[bernardoaraujor]

We should agree on terminology ASAP.

these are plugin interfaces:

• plugins/pap/pap_plugin.{c,h}
• plugins/pep/pep_plugin.{c,h}
• plugins/pip/pip_plugin.{c,h}

these are examples of plugin implementations:

• plugins/pep/implementation/pep_plugin_relay.{c,h}
• plugins/pip/implementation/pep_plugin_wallet.{c,h}
• plugins/pap/implementation/pap_plugin_rpi_storage.{c,h}

[strahinjagolic]

@bernardoaraujor Yes, in this case, PAP can have multiple plugins, with one interface.

[djordjeglbvc]

these are plugin interfaces:

* `plugins/pap/pap_plugin.{c,h}`

* `plugins/pep/pep_plugin.{c,h}`

* `plugins/pip/pip_plugin.{c,h}`

We wouldn't even need p*p_plugin.c, just p*p_plugin.h

[bernardoaraujor]

@strahinjagolic if we decide we need only one plugin for PAP, pluginmanager_get step isn't even necessary

@djordjeglbvc it's best to leave the possibility for multiple PAP plugin implementations... like I said above, you might want to store the policy as a UNIX-like file, or maybe as some bare metal non-volatile memory, or even on secure storage.

[bernardoaraujor]

We wouldn't even need pp_plugin.c, just pp_plugin.h

I agree, that's true. But let's do pxp_plugin.h to avoid special characters on the filename.

[djordjeglbvc]

@bernardoaraujor sure, but that could be just a run-time choice, user could choose one single plugin which they would use for PAP (choice is made in main.c), no need to have more than one for this use case. Doesn't hurt to leave that as an option, anyway.

We wouldn't even need p_p_plugin.c, just p_p_plugin.h

I agree, that's true. But let's do pxp_plugin.h to avoid special characters on the filename.

I used asterisk as a wildcard, we would need .h file for each plugin consumer module, for instance

plugins/pap/pap_plugin.h
plugins/pap/pep_plugin.h
plugins/pap/pip_plugin.h

but we don't need .c file for interface.