I think this is a good feature, and one which has been requested for a long time, so thanks for taking the time to work on it.

Before going deeper into review, I'd like to ask for more details on why you chose to use the obsolete gadcon API over either the gadget API or sandbox infrastructure. While you mention "that is what i use etc.", this is not consistent with the development direction of the community/project for the past 2+ years. In fact, all gadcon gadgets have since gained gadget equivalents, and there are a number of gadgets which either were not written to include gadcon implementations or have since removed them.

At this time, I'm opposed to adding any new gadgets in-tree which implement only the gadcon API as this infrastructure has been unmaintained and inactive for years despite having numerous known bugs. It seems that the sandbox infrastructure would be better suited as this gadget does not require access to enlightenment internals and it would simplify the gadget code considerably. I'm sure @stephenmhouston would be willing to help out with this since he's done considerable work in this area.

I'd like to ask for more details on why you chose to use the obsolete gadcon API

i use shelves. still. they are the stable still public and supported api. they work and work well and i want and need something that works for me now rock solidly. i also started with the bluez4 code and stripped it down to just the gadget then built on top thus that's how it started. 95% or so of the code has nothing to do with gadcon/gadgets so its shareable between both. it can be added. i'm not against that at all and am willing to do it myself, but i already had a few people ask to share this and use it and play with it etc. and people who just plain need it, so the sooner it goes in, the better. new gadget support can be added no problem. but i don't see that as necessary right now. i have tested this across several devices and think i have gotten this good but i'm not totally sure if it works for others as well. sharing will determine that and i'm pretty sure those people asking me to share it are still on shelves too. :)

despite having numerous known bugs

i spend my days and nights and weeks without seeing them. it's stable and very usable. i have tried bryces and found them to be ... not so great and littered with far more bugs than shelves (from usability bugs to visual snags to segfaults). i'll leave it at that.

It seems that the sandbox infrastructure would be better suited

absolutely not. that would add a fat memory footprint overhead for something that runs all day long doing very little other than waiting for agent requests sometimes from bluez and the odd occasional paring or unpairing of some device. consuming all the overhead of a process (which is multiple mb's) for essentially doing nothing all day offends me to no end. it's a massive waste of resources. any functionality that is similar (network/wireless controls for example) is in the same boat. same for clocks, temp, cpu etc. gauges etc.

simplify the gadget code considerably

it wouldn't simplify really worth talking about. it'd remove some code from the providing of the gadget and gadcon iface handling, but then add extra code to re-do the popups that gadcon could do with creating ctxpopups and placing (minus ~5-10 lines on one gadcon side, plus ~10 lines on the popup side). we'd be about even given just the rest of the code related to popup (not its content) and gadcon.

so all in all i can add the gadget support, but i'd rather not iterate over this to do it. it's simple enough to read e.g. time module code to see how it works. i'll put it in once i can also land some other fixes i now also have pending because i have to be on a branch for this.

I don't mind helping you convert it to the E gadget. Just let me know if you want the help.

@stephenmhouston - thanks. i'll be happy to ask for advice, but it's the kind of thing i like to do. i haven't done it before, but i've read the relevant code, so if i run into problems i'll definitely ping. but i'm happy to do this as it is ultimately something that needs doing but wasn't my priority (getting bz5 to work and work well is and was and to dot my i's and cross my t's). there's lots of other things that still need doing like my points in the log and it needs wide testing.

I've taken some time to do a moderately deep review of this. Overall it looks good, though I've found some problem areas which can be improved.

I'm still opposed to merging this gadget to master until either a comparable e_gadget implementation exists or a sandbox port is created. You've indicated some dissatisfaction with sandboxing--which is understandable since there is still work pending which addresses the points you raised--so I'll be anticipating the e_gadget version either in another submitted patch using git-phab or merged into this patch. Until that time, this seems like a fine candidate for a feature branch testing.

It seems that bluez4 is dead. Is there any point in keeping the corresponding module or will you be following up with another patch to remove it?

I don't see 4. in your list of remaining issues as a blocker to eventually merging this, but I think a task should be created for it (and possibly the rest of the items) to ensure that it isn't forgotten since this is a significant feature.

Thanks again for taking the time to tackle this, as it is definitely a much-requested feature.

I'm still opposed to merging this gadget to master until either a comparable e_gadget implementation exists

does it matter? what is the value? there is no "we must keep clean history" policy here. i can add it later. i agreed to adding the e gadget support. you don't trust me to add it? this is heavily hurting my workflow right now and really getting under my skin. don't forget... i founded this project, wrote more code than most and you now want to treat me like i'm untrusted. i'm being courteous with a heads up on this.

or a sandbox port is created

"hell no". in fact notice here to you specifically: any e modules or gadgets i find using external process sandboxes will be reverted or nuked unless there is an incredibly good reason for an external process (e.g. a needs to sandbox for privacy/security or something else justifiable when push comes to shove). i will not have enlightenment become a bloated memory swamp of processes. i worked hard over many years to ensure e is not bloated. i will not see it undone.

i'm pushing back really hard here because it seems you didn't get the message earlier when this appeared... the memory will bloat per process by multiple mb and quicklaunch will never solve this. threads+fork are incompatible. read https://thorstenball.com/blog/2014/10/13/why-threads-cant-fork/ ... not to mention fd's being a nightmare. and by fork i mean fork + keep running, not fork + then exec soon after. it's not going to solve it. you haven't explained how i or the above article are so wrong and that it will. you'll remove all threads from efl? something else? the more threads - the worse. in fact just 1 already calls it quits on this (and we have multiple in efl now as core features). you've made your point about quicklaunch saving the world. it's not going to for starters for the reasons above.

You've indicated some dissatisfaction with sandboxing

it's not dissatisfaction. it's outright unbridled and vehement opposition to it for many reasons i have given. the work pending won't address the points raised - see above. threads are going to only be used more and more to get stalls out of the main loop and to make use of more cores and it's not solvable without effectively neutering the gains of sharing dirty pages (you have far fewer dirty pages and few if any shared data structs like modules), so it's still bloated.

It seems that bluez4 is dead. Is there any point in keeping the corresponding module or will you be following up with another patch to remove it?

undecided. i don't know if anyone happens to use it still, and the voices of discontent from users who got upgraded to bluez5 and didn't want it would be the indicator. at this point i see it as orthogonal but mildly related.

I don't see 4. in your list of remaining issues as a blocker to eventually merging

don't take that todo list as things i must to do merge. they are just things i must do and iw as planning on doing over time when this lands. i don't intend to create any tasks. i know what i have to do.

Thanks again for taking the time to tackle this, as it is definitely a much-requested feature.

you're welcome. so far i'm not thrilled with this review process. it's pitting a stick in my spokes. i'll address what i say i will, but i want to get on with doing things.