You are reading a single comment by @fanoush and its replies. Click here to read the full conversation.
  • I should add that jswrap_bangle.c already supports quite a wide variety of different hardware (more than when the first extra watches were being added) so it may be that not a great deal of extra changes are needed to it to support the watch you're interested in... Also if it's a matter of adding extra hardware and it doesn't add too much bulk (things like accelerometers are usually very easy to add) then I'm open to pulling that into the main Espruino repo.

  • it may be that not a great deal of extra changes are needed to it to support the watch you're interested in..

    Well, it may be not a great deal for you to navigate ~5700 line file and sprinkle it with few lines in ~10 places of that file for adding support for yet another accelerometer device without breaking anything.

    For people like me, @jeffmer, @yngv126399 it is significantly harder to do both parts (1. add, 2. not break). And I think we actually don't need to care about maybe 3/4 of that file to add another watch and yet for now we need to learn all of it.

    I think jswrap_bangle.c is the wrong file to modify when we want to add e.g. another 3 types of accelerometers or touchscreens.

    Currently if I want to add bma420,bma223,sc7a20 there are couple of places like this one
    https://github.com/espruino/Espruino/blo­b/master/libs/banglejs/jswrap_bangle.c#L­1270
    Where I need to extend this spaghetti of ifdefs with three more cases (and BTW bma223 is 8 bit while others may be 12bit). When I search for ACCEL_ on that page it is all over the place. Then I get to place like this https://github.com/espruino/Espruino/blo­b/master/libs/banglejs/jswrap_bangle.c#L­3199
    and need to solve it somehow for my watch, and BTW the bma223 in my watch is connected over spi, not i2c.

    Or another random example - with new watch I need to think about this line https://github.com/espruino/Espruino/blo­b/master/libs/banglejs/jswrap_bangle.c#L­1506 whether my watch is like F18 or not (whatever F18 actually is, I may not know). There are many place like this there.

    Basically just for adding one watch out of many I would need to add lot hacks/workarounds/ifdefs there, all the time thinking about all the other watches and hardware combinations already there. So it is not about adding one accelerometer type.

    With rest of Espruino it is like
    jswrap_storage.c (or jswrap_flash.c) calls into jsflash.c which calls into jshardware.c (jshFlashRead/Write/Erase...). When porting to another platform I can basically do just another jshardware with few jshFlash methods without thinking how storage of jsflash layer works.

    For bangle_jswrap.c all those layers are not there so generic Bangle watch policy/logic is intermixed with specific hardware support. It would be much easier and scalable to implement new accelerometer X support in separate file with known interface without touching jswrap_bangle.c at all. Same for new touchscreen , display, hr sensor or gps.

About

Avatar for fanoush @fanoush started