Introduce convertible module #49

Merged
raster merged 35 commits from rafspiny/enlightenment:master into master 2024-03-12 07:20:21 -07:00
Contributor

Close #43

Close #43
raster requested review from raster 2023-08-02 03:04:26 -07:00
raster requested changes 2023-08-02 04:37:54 -07:00
raster left a comment
Owner

See inline comments... only going to mentuion the same issue the first 1 or 2 times - same applies to other instances of it :)

See inline comments... only going to mentuion the same issue the first 1 or 2 times - same applies to other instances of it :)
@ -0,0 +7,5 @@
#include "dbus_acceleration.h"
#include "e_mod_main.h"
#include "input_rotation.h"
DbusAccelerometer* accelerometer_dbus;
Owner

shouldnt accelerometer_dbus be static?

shouldnt accelerometer_dbus be static?
Author
Contributor

Made it static.

Made it static.
@ -0,0 +9,5 @@
#include "input_rotation.h"
DbusAccelerometer* accelerometer_dbus;
DbusAccelerometer* sensor_proxy_init() {
Owner

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)

also some funcs are

int func(int x)
{
}

some are

int
func(int x)
{
}

with return type on previous line - be consistent and maybe do the latter?

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :) also some funcs are int func(int x) { } some are int func(int x) { } with return type on previous line - be consistent and maybe do the latter?
Author
Contributor

Corrected the linting for function to be

int
func(int x)
{
}

Also in the headers

int
func(int x);

Do we have a linting configi file somewhere? So we can get this kind of comments automatically? Like clang

Corrected the linting for function to be ``` int func(int x) { } ``` Also in the headers ``` int func(int x); ``` Do we have a linting configi file somewhere? So we can get this kind of comments automatically? Like clang
@ -0,0 +16,5 @@
{
INF("We already have a struct filled");
return accelerometer_dbus;
}
accelerometer_dbus = calloc(1, sizeof(DbusAccelerometer));
Owner

if calloc fails?

if calloc fails?
Author
Contributor

I think in most case I can use EINA_SAFETY_ON_NULL_RETURN_VAL. Am I correct?

I think in most case I can use EINA_SAFETY_ON_NULL_RETURN_VAL. Am I correct?
@ -0,0 +22,5 @@
// The next line is probably redundant
accelerometer_dbus->orientation = undefined;
DBG("Before eldbus initialization");
int initialization = eldbus_init();
Owner

do we really need this? eldnbus should alredy be initted by e before any module loads...

do we really need this? eldnbus should alredy be initted by e before any module loads...
Author
Contributor

Removed it.

Removed it.
@ -0,0 +57,5 @@
return accelerometer_dbus;
}
void sensor_proxy_shutdown()
Owner

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)
Author
Contributor

I think I fixed all of them.

I think I fixed all of them.
@ -0,0 +109,5 @@
return sensor_proxy;
}
enum screen_rotation access_string_property(const Eldbus_Message *msg, Eldbus_Message_Iter **variant, Eina_Bool* result)
Owner

wouldnt this be better to just have one of the screen_orientation enum type of undefined that is already there and not have a bool result return ptr arg?

btw - in e/efl we dont use enum xxxx we typedef like typedef enum Screen_Orientation Screen_Orientation

also functions like access_string_property are only accessed within this file - shouldnt they be static? and maybe be named access_string_property (with an underscore at the front) to show they are local only to that file? there are various other funcs that are not needed outside of this file .. so consider moving them all to static?

wouldnt this be better to just have one of the screen_orientation enum type of undefined that is already there and not have a bool result return ptr arg? btw - in e/efl we dont use enum xxxx we typedef like typedef enum Screen_Orientation Screen_Orientation also functions like access_string_property are only accessed within this file - shouldnt they be static? and maybe be named _access_string_property_ (with an underscore at the front) to show they are local only to that file? there are various other funcs that are not needed outside of this file .. so consider moving them all to static?
Author
Contributor
  1. Yes, I just got rid of the boolean. Using the defaule enum value of UNDEFINED
  2. I will adapt that in the next commit
  3. On my list
1) Yes, I just got rid of the boolean. Using the defaule enum value of UNDEFINED 2) I will adapt that in the next commit 3) On my list
@ -0,0 +111,5 @@
}
enum screen_rotation access_string_property(const Eldbus_Message *msg, Eldbus_Message_Iter **variant, Eina_Bool* result)
{
const char *type = NULL;
Owner

this should not be cosnt... :)

this should not be cosnt... :)
Author
Contributor

Fixed.

Fixed.
@ -0,0 +138,5 @@
{
WARN("Expected type is string(s).");
*result = EINA_FALSE;
}
const char **string_property_value = calloc(PATH_MAX, sizeof(char));
Owner

if calloc fails?

if calloc fails?
Author
Contributor

Same thing. I think in most case I can use EINA_SAFETY_ON_NULL_RETURN_VAL. Am I correct?

Same thing. I think in most case I can use EINA_SAFETY_ON_NULL_RETURN_VAL. Am I correct?
@ -0,0 +146,5 @@
*result = EINA_FALSE;
}
enum screen_rotation rotation = undefined;
if (strcmp(ACCELEROMETER_ORIENTATION_RIGHT, *string_property_value) == 0)
Owner

shouldnt all the below if's be else if () ? also e/efl use the if (!strcmp()) convention :)

shouldnt all the below if's be else if () ? also e/efl use the if (!strcmp()) convention :)
Author
Contributor

Changed all of them.

Changed all of them.
@ -0,0 +155,5 @@
rotation = flipped;
if (strcmp(ACCELEROMETER_ORIENTATION_NORMAL, *string_property_value) == 0)
rotation = normal;
free((void *) type);
Owner

you dont have to cast to void * for free in C.

you dont have to cast to void * for free in C.
Author
Contributor

Thanks. Remove the cast in all 3 places.

Thanks. Remove the cast in all 3 places.
@ -0,0 +163,5 @@
Eina_Bool
access_bool_property(const Eldbus_Message *msg, Eldbus_Message_Iter **variant, Eina_Bool *boolean_property_value)
{
const char *type;
Owner

shouldn't be const... :)

shouldn't be const... :)
Author
Contributor

Done.

Done.
rafspiny marked this conversation as resolved
@ -0,0 +194,5 @@
{
WARN("error in eldbus_message_iter_arguments_get()");
res = EINA_FALSE;
}
free((void *) type);
Owner

no need for void * cast

no need for void * cast
rafspiny marked this conversation as resolved
@ -0,0 +211,5 @@
ERR("Error: %s %s", errname, errmsg);
}
access_bool_property(msg, &variant, &has_accelerometer);
DbusAccelerometer *accelerometer = (DbusAccelerometer *) data;
Owner

dont need to cast to DbusAccelerometer * ... C casts anything to/from void * automatically as void * accepts any ptr (well const is another matter - separate).

dont need to cast to DbusAccelerometer * ... C casts anything to/from void * automatically as void * accepts any ptr (well const is another matter - separate).
Author
Contributor

Removed cast

Removed cast
@ -0,0 +232,5 @@
const char *errname, *errmsg;
enum screen_rotation orientation;
Eldbus_Message_Iter *variant = NULL;
Eina_Bool* result = calloc(1, sizeof(Eina_Bool));
Owner

waiiit.. whay do we calloc a bool? why? why not just have it on the stack?

waiiit.. whay do we calloc a bool? why? why not just have it on the stack?
Author
Contributor

I got rid of result

I got rid of `result`
@ -0,0 +262,5 @@
EINA_LIST_FOREACH(inst->randr2_ids, l, randr_id)
{
_fetch_and_rotate_screen(randr_id, orientation);
}
free((void *)randr_id);
Owner

this doersn't make sense... to free this when it's a const char * and you loop over the list stored in randr2_ids ? this smells of all kinds of wrong and bugs to me... i haven't seen how you create this list of strings yet...

this doersn't make sense... to free this when it's a const char * and you loop over the list stored in randr2_ids ? this smells of all kinds of wrong and bugs to me... i haven't seen how you create this list of strings yet...
Author
Contributor

Ok. This should not be const. Removed that.

Ok. This should not be const. Removed that.
@ -0,0 +298,5 @@
}
return transformation;
}
int _fetch_X_device_input_number()
Owner

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)

also shouldnt this be static - like many others. it's local only to this file?

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :) also shouldnt this be static - like many others. it's local only to this file?
@ -0,0 +302,5 @@
int _fetch_X_device_input_number()
{
// I should get the touchscreen associated with the screen probably by looking at the classes of the input devices
// I need to submit my patch to add getters for other XIDeviceInfo fields, like raster mentioned in his commit.
const char *dev_name = NULL;
Owner

don't have to set to NULL... always set below

don't have to set to NULL... always set below
Author
Contributor

Done, for both lines.

Done, for both lines.
rafspiny marked this conversation as resolved
@ -0,0 +303,5 @@
{
// I should get the touchscreen associated with the screen probably by looking at the classes of the input devices
// I need to submit my patch to add getters for other XIDeviceInfo fields, like raster mentioned in his commit.
const char *dev_name = NULL;
char **property_name = NULL;
Owner

same as above - dont need to set to NULL

same as above - dont need to set to NULL
rafspiny marked this conversation as resolved
@ -0,0 +307,5 @@
char **property_name = NULL;
int dev_num = ecore_x_input_device_num_get();
int dev_number = -1;
for (int dev_counter=0; dev_counter<dev_num; dev_counter++)
Owner

stytle-wise this is mixed. most of e/efl uses spaces like 'int x = 0' not 'int x=0'

stytle-wise this is mixed. most of e/efl uses spaces like 'int x = 0' not 'int x=0'
Author
Contributor

Done.

Done.
@ -0,0 +320,5 @@
int is_correct_device = _is_device_a_touch_pointer(dev_counter, num_properties, iterator);
if (is_correct_device == EINA_FALSE)
continue;
iterator = property_name;
for (int i=0; i<num_properties; i++)
Owner

spaces as above

spaces as above
Author
Contributor

Done.

Done.
rafspiny marked this conversation as resolved
@ -0,0 +322,5 @@
continue;
iterator = property_name;
for (int i=0; i<num_properties; i++)
{
if (strcmp(*iterator, CTM_name) == 0)
Owner

!strcmp...

!strcmp...
Author
Contributor

Changed all 5 of them.

Changed all 5 of them.
@ -0,0 +346,5 @@
if (strstr(*iterator, "Axis Labels") != NULL)
{
int num_ret, unit_size_ret;
Ecore_X_Atom format_ret;
char *result = NULL;
Owner

int result = ecore_x.... ? better

int result = ecore_x.... ? better
Author
Contributor

char* result = ecore_x...

char* result = ecore_x...
raster marked this conversation as resolved
@ -0,0 +348,5 @@
int num_ret, unit_size_ret;
Ecore_X_Atom format_ret;
char *result = NULL;
result = ecore_x_input_device_property_get(dev_counter, *iterator, &num_ret, &format_ret, &unit_size_ret);
if (result != NULL) {
Owner

shorter:

if (result)  ...
shorter: if (result) ...
Author
Contributor

Fixed all (xxx != NULL) conditions.

Fixed all `(xxx != NULL)` conditions.
@ -0,0 +386,5 @@
int num_ret, unit_size_ret;
Ecore_X_Atom format_ret;
char *result = NULL;
TransformationMatrix *matrix = calloc(1, sizeof(TransformationMatrix));
Owner

calloc fails?

calloc fails?
Author
Contributor

Same thing. I think in most case I can use EINA_SAFETY_ON_NULL_RETURN_VAL. Am I correct?

Same thing. I think in most case I can use EINA_SAFETY_ON_NULL_RETURN_VAL. Am I correct?
@ -0,0 +9,5 @@
#define EFL_DBUS_ACC_PATH "/net/hadess/SensorProxy"
#define EFL_DBUS_ACC_IFACE "net.hadess.SensorProxy"
// This enum represents the 4 states of screen rotation plus undefined
enum screen_rotation {undefined, normal, right_up, flipped, left_up};
Owner

normally er ur all caps for enums like UNDEFINED NORMAL etc.

normally er ur all caps for enums like UNDEFINED NORMAL etc.
Author
Contributor

Made them all caps.

Made them all caps.
@ -0,0 +43,5 @@
* @param result 1 if result is ok, 0 if it failed
* @return Enum specifying the orientation
*/
enum screen_rotation
access_string_property(const Eldbus_Message *msg, Eldbus_Message_Iter **variant, Eina_Bool* result);
Owner

wouldnt a lot fo funcs like this just be static internal to the .c file and not in the .h?

wouldnt a lot fo funcs like this just be static internal to the .c file and not in the .h?
@ -0,0 +7,5 @@
/* LIST OF INSTANCES */
static Eina_List *instances = NULL;
void _update_instances(const Instance *current_instance)
Owner

shouldnt this be a static func? same with many others...

shouldnt this be a static func? same with many others...
@ -0,0 +138,5 @@
return cfd;
}
void
econvertible_config_init()
Owner

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)
Author
Contributor

I think I addressed them all now.

I think I addressed them all now.
@ -0,0 +152,5 @@
DBG("Config loaded");
}
void econvertible_config_shutdown()
Owner

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)

functions in C that take NO arguments are (void) not (). () means "i accept any number of arguments of an undefined type". :)
Author
Contributor

I think I addressed them all now.

I think I addressed them all now.
@ -0,0 +289,5 @@
if (!accelerometer->dbus_property_changed_sh)
ERR("Error: could not add the signal handler for PropertiesChanged");
// Screen related part
E_Zone *zone = NULL;
Owner

don't have to set to NULL

don't have to set to NULL
Author
Contributor

Done.

Done.
@ -0,0 +306,5 @@
if (screen->info.can_rot_90 == EINA_TRUE)
{
int max_screen_length = 300;
char *randr2_id = malloc(sizeof(char) * max_screen_length);
int copied_chars = eina_strlcpy(randr2_id, zone->randr2_id, max_screen_length);
Owner

why al of the above? why not just strdup? :)

why al of the above? why not just strdup? :)
Author
Contributor

I thing I tried 2 years ago and got some weird issue. Re-introducing it.

I thing I tried 2 years ago and got some weird issue. Re-introducing it.
raster marked this conversation as resolved
rafspiny changed title from Introduce convertible module to WIP: Introduce convertible module 2023-08-03 12:34:41 -07:00
Author
Contributor

@raster I made some changes to address your comments. Ready to give it another go.

@raster I made some changes to address your comments. Ready to give it another go.
raster requested changes 2023-11-14 09:00:04 -08:00
raster left a comment
Owner

See inline comments. :) there's a bit of consistency work to be done as a general thing - like consistently choosing the same var names e.gh. one place had Evas_Object *o; and one Evas_Object *evas_object;

See inline comments. :) there's a bit of consistency work to be done as a general thing - like consistently choosing the same var names e.gh. one place had Evas_Object *o; and one Evas_Object *evas_object;
@ -0,0 +60,5 @@
int num_ret, unit_size_ret;
Ecore_X_Atom format_ret;
char *result = ecore_x_input_device_property_get(dev_counter, *iterator, &num_ret, &format_ret, &unit_size_ret);
if (result) {
// TODO Shall check for the value "Abs MT Position"
Owner

You should free result.... or you will be leaking.

You should free result.... or you will be leaking.
raster marked this conversation as resolved
@ -0,0 +99,5 @@
{
if (!strcmp(*iterator, CTM_name))
{
dev_number = dev_counter;
DBG("Setting device: %d", dev_number);
Owner

Any reason ytou keep looping and don't just break the loop here?

Any reason ytou keep looping and don't just break the loop here?
raster marked this conversation as resolved
@ -0,0 +161,5 @@
memcpy(matrix->values, rotation_matrix_2d, 6 * sizeof(*rotation_matrix_2d));
ecore_x_input_device_property_set(x_dev_num, CTM_name, matrix->values, num_ret, format_ret, unit_size_ret);
DBG("Input device %d rotated to %d", x_dev_num, rotation);
} else {
Owner

free result again as above.

free result again as above.
raster marked this conversation as resolved
@ -0,0 +227,5 @@
res = EINA_FALSE;
return res;
}
if (type[1])
Owner

correctness. checking type[1] without type[0] checked first is assuming type will never be "" (empty string) as then type[1] wll be beyond the end of this string. shouldn't you restructure this to be sure type[0] != 0 first so type[1] will exist?

correctness. checking type[1] without type[0] checked first is assuming type will never be "" (empty string) as then type[1] wll be beyond the end of this string. shouldn't you restructure this to be sure type[0] != 0 first so type[1] will exist?
raster marked this conversation as resolved
@ -0,0 +405,5 @@
return rotation;
}
type = eldbus_message_iter_signature_get((*variant));
if (type[1])
Owner

same here again - checking type[1] before type[0] is known to be 0 or not.

same here again - checking type[1] before type[0] is known to be 0 or not.
@ -0,0 +413,5 @@
if (type[0] != 's')
{
WARN("Expected type is string(s).");
}
const char **string_property_value = calloc(PATH_MAX, sizeof(char));
Owner

this calloc jusyt screams "wrong". char **x = calloc(...); ... as opposed to char *x = calloc(...);

this calloc jusyt screams "wrong". char **x = calloc(...); ... as opposed to char *x = calloc(...);
raster marked this conversation as resolved
@ -0,0 +438,5 @@
void
on_accelerometer_orientation(void *data, const Eldbus_Message *msg, Eldbus_Pending *pending EINA_UNUSED)
{
INF("New orientation received");
Instance *inst = (Instance *) data;
Owner

youy know you don't have to cast to/from void * right... ? everything auto-casts to/from void * ... :)

youy know you don't have to cast to/from void * right... ? everything auto-casts to/from void * ... :)
@ -0,0 +11,5 @@
static void
_update_instances(const Instance *current_instance)
{
Eina_List *l;
Instance *instance = NULL;
Owner

you know you dont have to set instance to NULL... :)

you know you dont have to set instance to NULL... :)
@ -0,0 +10,5 @@
// Definition of the data structure to hold the gadget configuration
typedef struct _Convertible_Config Convertible_Config;
struct _Convertible_Config
{
E_Module *module;
Owner

And reasons module is here inb config structs and not just a global for the module for the handle to the module data? you have convertible_module already as a global...

And reasons module is here inb config structs and not just a global for the module for the handle to the module data? you have convertible_module already as a global...
@ -0,0 +216,5 @@
static Evas_Object *
_gc_icon(const E_Gadcon_Client_Class *client_class EINA_UNUSED, Evas *evas)
{
Evas_Object *o;
char buf[PATH_MAX];
Owner

consistency - here its PATH_MAX = below its 4096... :)

consistency - here its PATH_MAX = below its 4096... :)
@ -0,0 +227,5 @@
static const char *
_gc_id_new(const E_Gadcon_Client_Class *client_class EINA_UNUSED)
{
static char buf[4096];
Owner

consistency - here it's 4096, above it's PATH_MAXw its 4096... :)

consistency - here it's 4096, above it's PATH_MAXw its 4096... :)
raster marked this conversation as resolved
raster reviewed 2024-01-16 04:06:42 -08:00
@ -0,0 +9,5 @@
#define EFL_DBUS_ACC_PATH "/net/hadess/SensorProxy"
#define EFL_DBUS_ACC_IFACE "net.hadess.SensorProxy"
// This enum represents the 4 states of screen rotation plus UNDEFINED
enum screen_rotation {UNDEFINED, NORMAL, RIGHT_UP, FLIPPED, LEFT_UP};
Owner

style-wise - in e and efl we never just use struct xxx or enum xxxx we typedef to a full type. it's just style/consistency.

style-wise - in e and efl we never just use struct xxx or enum xxxx we typedef to a full type. it's just style/consistency.
Author
Contributor

Changed into typedef enum {UNDEFINED, NORMAL, RIGHT_UP, FLIPPED, LEFT_UP} screen_rotation;
Adapted the type whenever it was used.

Changed into typedef enum {UNDEFINED, NORMAL, RIGHT_UP, FLIPPED, LEFT_UP} screen_rotation; Adapted the type whenever it was used.
raster reviewed 2024-01-16 04:07:45 -08:00
@ -0,0 +25,5 @@
/**
* Fetch the DBUS interfaces and fill the DbusAccelerometer struct
* */
DbusAccelerometer* sensor_proxy_init();
Owner

func() in c means zero OR more arguments - unknown. (void) means no arguments at all. different to c++ and other langs.

func() in c means zero OR more arguments - unknown. (void) means no arguments at all. different to c++ and other langs.
Author
Contributor

Done

Done
rafspiny marked this conversation as resolved
raster reviewed 2024-01-16 04:07:51 -08:00
@ -0,0 +28,5 @@
* */
DbusAccelerometer* sensor_proxy_init();
void sensor_proxy_shutdown();
Owner

func() in c means zero OR more arguments - unknown. (void) means no arguments at all. different to c++ and other langs.

func() in c means zero OR more arguments - unknown. (void) means no arguments at all. different to c++ and other langs.
Author
Contributor

Done

Done
rafspiny marked this conversation as resolved
raster reviewed 2024-01-16 04:12:16 -08:00
@ -0,0 +45,5 @@
{
DBG("Keyboard: Signal %s received from %s", sig, src);
}
void
Owner

The way you use this later... you could just make Eina_List *instances declared in the e-gadget-convertible.h file instead of static and lose this function entirely... it'd be less code.

The way you use this later... you could just make Eina_List *instances declared in the e-gadget-convertible.h file instead of static and lose this function entirely... it'd be less code.
Author
Contributor

I removed the function and moved instances to the header. Also removed static. That should do.

I removed the function and moved `instances` to the header. Also removed static. That should do.
Author
Contributor

@raster should it not be static? It is used in e_mod_main.c too.

@raster should it not be static? It is used in e_mod_main.c too.
Owner

If you expose the Eina_List *list; then it shouldn't be static. static == local to that file only and not visible outside of that file (for functions and global vars. for vars within a function it means they keep their value between calls to that function).

If you expose the Eina_List *list; then it shouldn't be static. static == local to that file only and not visible outside of that file (for functions and global vars. for vars within a function it means they keep their value between calls to that function).
raster reviewed 2024-01-16 04:20:04 -08:00
@ -0,0 +9,5 @@
// Definition of the data structure to hold the gadget configuration
typedef struct _Convertible_Config Convertible_Config;
struct _Convertible_Config
{
Owner

If you look at other config structs = i normally put an int version in in advance. this allows me to version the config and if in future new thing are added or removed ... the version can help clear up how old the config file is and "upgrade it". i'd suggest putting that in and well - removing the module field as its not part of config really :)

If you look at other config structs = i normally put an int version in in advance. this allows me to version the config and if in future new thing are added or removed ... the version can help clear up how old the config file is and "upgrade it". i'd suggest putting that in and well - removing the module field as its not part of config really :)
raster reviewed 2024-01-16 04:23:55 -08:00
@ -0,0 +83,5 @@
// evas_object_smart_callback_add(parent, "gadget_site_anchor", _anchor_change, inst);
// Adding callback for EDJE object
INF("Adding callback for creation and other events from EDJE");
edje_object_signal_callback_add(evas_object, "lock,rotation", "tablet", _rotation_signal_cb, instance);
Owner

ok - this is an edje signal api thing. in e all signals he a "source" of "e" for namspacing. this allows you to differentiate between signals that are just sent within an edje obj from part to part and signals that go to/from e. you should probably adapt the edj to use "e" source namespaced signals to be clean.

ok - this is an edje signal api thing. in e all signals he a "source" of "e" for namspacing. this allows you to differentiate between signals that are just sent within an edje obj from part to part and signals that go to/from e. you should probably adapt the edj to use "e" source namespaced signals to be clean.
raster reviewed 2024-01-16 07:01:44 -08:00
@ -0,0 +359,5 @@
DBG("Removing the logger");
eina_log_domain_unregister(_convertible_log_dom);
_convertible_log_dom = -1;
return 1;
Owner

missing e_gadcon_provider_unregister(); here - you don't unregister the gadcons on module unload. you'll crash and not delete the convertible gadgets if you unload the module (try unloading the module with convirtble gadgets floating about...)/

missing e_gadcon_provider_unregister(); here - you don't unregister the gadcons on module unload. you'll crash and not delete the convertible gadgets if you unload the module (try unloading the module with convirtble gadgets floating about...)/
Author
Contributor

I do use it. Right before unregistering the log domain.

INF("Shutting down the module");
   convertible_module = NULL;
   e_gadcon_provider_unregister(&_gadcon_class);
I do use it. Right before unregistering the log domain. ``` INF("Shutting down the module"); convertible_module = NULL; e_gadcon_provider_unregister(&_gadcon_class); ```
rafspiny marked this conversation as resolved
rafspiny changed title from WIP: Introduce convertible module to Introduce convertible module 2024-02-23 15:49:13 -08:00
Author
Contributor

@raster I addressed your comments. It would be great to have another review from you whenever you have some spare time.

@raster I addressed your comments. It would be great to have another review from you whenever you have some spare time.
raster approved these changes 2024-03-12 07:19:11 -07:00
raster force-pushed master from efa2c17235 to 3def50bd6a 2024-03-12 07:20:10 -07:00 Compare
raster merged commit 3def50bd6a into master 2024-03-12 07:20:21 -07:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: enlightenment/enlightenment#49
No description provided.