You are reading a single comment by @Gordon and its replies. Click here to read the full conversation.
  • I REALLY NEED TO HEAR OTHER PEOPLE'S THOUGHTS ON THIS...

    @user158096 and @fanoush I think you're the only ones that have expressed interest in this so far? Please can you have a look at https://github.com/brendena/Espruino/tree/MOD_DISPLAY/libs/banglejs/hardware/backlight where the display backlight code has been split out, and imagine that there's an issue setting the backlight brightness on the Bangle.js 2 with setLCDPowerBacklight (or anything else, like maybe adding a new watch), and let me know, honestly, whether you think it's easier to find/change the code in the refactored repo, or the old one?

    Being totally honest, every time I see a message about these changes, it just fills me with dread... I thought I was just being averse to change and not giving it a chance, so I thought I'd give it a go to see if it really was easier to work on than the current code.

    So... I started looking, trying the above. I found jswrap_banglejs_setLCDBrightness in one file (which is fine - that would automatically link from the reference pages anyway) , and that calls jswrap_banglejs_setLCDPowerBacklight, which calls banglejs_setLCDPowerBacklight_impl (which isn't documented) and that calls banglejs_pwrBacklight_impl (which isn't documented either) sometimes, but I'm not sure for what boards it does that or which file is for which device, and banglejs_pwrBacklight_impl is empty. I do see banglejs_setLCDPowerBacklight_impl in bangle_backlight_f18_impl.c, and that does set the brightness, but for Bangle.js 1. And backlight_fade sets it too.

    banglejs_setLCDPowerBacklight_impl in jswrap_bangle_backlight.c does have a #ifdef LCD_BL that sets the brightness, but it's also set in bangle_backlight_impl.c as well (actually I think that was only in the MOD_BACKLIGHT branch which I realise now is older), and the pin is set in bangle_backlight_LPM013M126_impl.c which I know is also Bangle.js 2 but isn't obvious - so I'm not sure which files actually do Bangle.js 2, but I have a sneaking suspicion that LCD_BL is now set from at least 2 separate files even on the latest branch (3 if using lcd_fade which we don't currently).

    And this is just for the backlight, that's a glorified LED. It's now implemented over 7 source files. I don't know what it'll look like for something more complex.

    Trying to find all those functions in different files on GitHub is a nightmare, and I guess it only really works if you've got VS Code and you can do a text search over the whole project for everything. I'm fully aware jswrap_bangle.c is big and people don't like it. It's a mess of legacy code and untangling it is a huge problem, but I'm not convinced taking that code, adding more indirection, and splitting it over multiple files is going to be better even if it does reduce the size of jswrap_bangle.c.

    I've spent hours this morning trying to go through this, and be honest with myself about it, but it's just doing my head in...

    I know you've spent ages on this, but I'm here wondering how on earth I'm going to maintain anything after these changes go in. Realistically I end up doing at least 90% of the work on this, and I feel like it's going to make my life much harder... not to mention something like the DICKENS/Starfield watch which was a project for a customer and which I think is very unlikely to work afterwards without a lot of unpaid work from me - I'll just have to take that out of the build and leave it as an unmaintained fork.

    ... and I don't really understand why we're doing this. We could support the P8 with just another few IFDEFs in jswrap_bangle.c and it wouldn't make it noticeably harder to understand than it is right now. Nobody's even asked me about PineTime/P8 in the last year anyway, and very few people contribute or have expressed interest in contributing to the main Bangle.js codebase.

    I feel like with the best will in the world, this is going to take weeks of my time vetting/testing and fixing regressions, it's going to add bugs for over 10,000 paying customers, and all to make it marginally easier for one of two people to use it (maybe) on a $25 watch. None of this will benefit me or 90% of actual users.

    I'm sorry, but I feel like I have to be honest about my feelings on this.

    I know you've put bags of work into this but I also don't want you to put in loads more if I can't bring myself to merge the changes in - and I'm looking at https://github.com/brendena/Espruino/tree/MOD_DISPLAY and just can't see how merging it or anything like it isn't going to cause me lots of grief.

    At the end of the day, I'm trying to make a living on software I'm giving away, and I feel like most days it's a massive struggle. The vast majority of work I do I don't ever get paid for - and I don't really have the luxury of just changing stuff for the sake of changing it, because with the best will in the world there will be issues which need sorting out, and at the end of the day it'll be me that has to deal with them...

About

Avatar for Gordon @Gordon started