Skip to content

Conversation

@RKBoss6
Copy link
Contributor

@RKBoss6 RKBoss6 commented Jan 20, 2026

Refactors for better responsiveness, reliability, toggle behaviour, ui look and feel, etc. Look at the changelog for a full description. Also, tagging @bobrippling as the author of this app.

Refactor Overlay and Controls for improved functionality and visuals.
Updated version number and enhanced description for clarity.
Updated the README to include additional features and clarify gesture handling.
Copilot AI review requested due to automatic review settings January 20, 2026 21:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the Control Pad app to improve UI consistency, reliability, and user experience. The changes transition from text-based controls to icon-based controls with a new rounded visual style, add a settings shortcut, and improve the toggle behavior for BLE, quiet mode, and HRM controls.

Changes:

  • Refactored control system from callback-based state management to separate get/toggle functions for better reliability
  • Updated UI with rounded borders, icons instead of text, and wider overlay for better visual appeal
  • Added a sixth control button for quick access to settings app

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 27 comments.

File Description
apps/ctrlpad/metadata.json Version bump to 0.03 and updated description to reflect new features
apps/ctrlpad/README.md Updated documentation to reflect reduced touch activation area (40px → 10px) and new shortcuts
apps/ctrlpad/ChangeLog Comprehensive changelog documenting all refactoring changes and new features
apps/ctrlpad/main.js Major refactor: new UI styling with rounded corners, icon-based controls, refactored toggle logic for BLE/DnD/HRM, added settings button

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RKBoss6 and others added 10 commits January 20, 2026 17:40
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@RKBoss6
Copy link
Contributor Author

RKBoss6 commented Jan 21, 2026

I want to add a screenshot, but I don't think g.dump() supports lcd overlays - any alternatives?

@bobrippling
Copy link
Collaborator

Hey, this looks great! One problem however - the app is typescript. Before you go any further, would you be able to migrate your changes to main.ts ?

For the screenshot, you'll want to call ui.overlay.g2.dump() - here after ui is assigned, you can copy it to global.ui or similar:

const overlay = new Overlay();
ui = {
overlay,
ctrls: new Controls(overlay.g2, controls),

@RKBoss6
Copy link
Contributor Author

RKBoss6 commented Jan 21, 2026

Thanks! Is there a way to generate typescript from the js code, or do I have to manually reimplement everything?

@bobrippling
Copy link
Collaborator

Thanks! Is there a way to generate typescript from the js code, or do I have to manually reimplement everything?

It's the other way round in fact, JS is generated from TS. You could try your hand at patching across but that's not likely to help much (git diff | sed 's/\.js/\.ts/' | patch -p1)

@RKBoss6
Copy link
Contributor Author

RKBoss6 commented Jan 21, 2026

Hmm, I don't know typescript (at all), so I'm afraid I won't be of much use. Do you think it's possible for you to make those changes? The other option is I can ask an AI to see if that'll work, but I'm not sure about that either. Perhaps I'm misunderstanding something fundamental with TypeScript though, but I'm just not sure how to go about this...

(the git diff | sed 's/\.js/\.ts/' | patch -p1 didn't work...)

RKBoss6 and others added 5 commits January 21, 2026 14:35
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Expanded the README with additional details about the app's design and contributors.
Copy link
Collaborator

@bobrippling bobrippling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I've moved the changes into main.ts, it doesn't compile yet but thought I'd comment a few thoughts before going further. Let me know what you think!

var border = 3;
this.g2
.reset()
.setColor(13)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the 13 all about here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah- it was just a number in the palette that's used as a transparent background color for the corners. It's the best solution I was able to come up with, but if you have another way, I would prefer having a more elegant solution over this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotya - I don't know of any way to set transparent otherwise. Do you have the source palette for us to see how likely it is that this will change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by source palette :( If it helps, in the screenshot before I edited the layers on, the transparent color was shown as an orange..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I mean just where the 13 comes from, like where you found it - the image bytes themselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, like I said before, it's just a random number I chose, with no significance at all. That probably isn't the best way to go about something like that, but I didn't find another way :(
Do we need to change it/actually source the value from somewhere? It has worked for all I've been using it, you can upload the boot.js to your watch as well if you need to test it.
(I could still be misunderstanding what you're trying to ask, I'm sorry for the confusion)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see! I think 13 is ok then, let's make it clear in a comment that it's a somewhat arbitrary variable chosen out of nowhere in particular :)

toggle: () => {
let s = require("Storage").readJSON("setting.json", 1);
s.quiet = s.quiet ? 0 : 1;
require("Storage").writeJSON("setting.json", s);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make this optional, I want the quiet mode to be temporary and also avoid the flash writes on each tap

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only gripe with that is when I first used the control panel, bluetooth wouldn't connect, and it took me a few days before I figured out that it was locally turning bluetooth off, and just wouldn't display that. I would prefer to have it global so all apps are affected but haptics still work, but possibly avoiding flash writes if we can.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a mix-up here? This is quiet mode, not bluetooth?

Copy link
Contributor Author

@RKBoss6 RKBoss6 Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I was using it as an example. The point was I didn't want any local change to quiet mode when there's already a global implementation. Could we have a global variable used in messages instead that is saved on e. Kill? I think that would be a better solution for system-level quiet mode, as it avoids writes unnecessarily until E.kill is triggered, and can just be appended in bootloader for simplicity.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see - my use case is different - I want a global change to just buzz but I don't want quiet mode installed or to have it as a hard dependency of the app. Perhaps have the code check if quiet mode scheduler or similar is installed and enable the functionality in that case?

And yes, agreed - saving quiet mode setting in the kill event handler sounds good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! For the e.kill handler, we'll need a global variable to act in the file's place, which we have to create. Should we do that, and then update all the apps that use quiet mode? Alternatively, we could have a more system level one, and create a Bangle.quietMode variable for the system instead that achieves the same effect. @gfwilliams wdyt?

Copy link
Member

@gfwilliams gfwilliams Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like the idea of starting to mess with other apps and adding global variables just to deal with this case. So the reason you want to save is because the messages lib looks at quiet in setting.json?

It looks like you and @bobrippling want different things, and as he says maybe you need it to be an option.

Either:

  • You have a temporary change which you can do by just overwriting the .buzz function (although if quiet was set already in settings, you won't ever be able to unset it by changing .buzz)
  • You change the global setting by writing to setting.json which I think is the best bet - I wouldn't worry about the kill handler (as then you've got these two ways of disabling buzzing which are overlapping). You're only writing in response to a user input which you'll do at most a few times a day, so I don't think this is a big deal.

I think probably the best bet it to switch to just changing .quiet, and then if it's temporary (as bob would like) in a kill handler you set the setting back to what it was before.

Interestingly internally Bangle.js actually reads the settings file each time an app is loaded and looks at the value of vibrate, and if so disables JSBF_ENABLE_BUZZ which stops vibration regardless of what you do to .buzz or what quiet is set to. Really, longer term we should probably just expose the ability to set that flag and delete all the checks for .quiet.

I guess quiet was added rather than changing vibrate because someone still wanted the haptics even when quiet, but maybe we could have a Bangle.haptic that the built-in menus use, and then that gives us a bit more flexibility (to disable haptics as well as making them stronger/weaker).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good! As for this however, I can make it a setting and use that to decide whether we want buzzes at all, or only quiet mode for messages, etc. It might take some time though, as I'm a bit busy now, but I'll try to get it done soon.

{
text: "HRM",
img:atob("GBiBAAAAAA+B8B/D+D/n/H///v/////////P///P///H/3+WQAC2Xj82HB86uA/48Af54AP9wAH9gAD/AAB+AAA8AAAYAAAAAAAAAA=="),
get: () => Bangle.isHRMOn(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will mislead users - tapping the button will have no visible effect if another app/widget has enabled it (it'll remain on)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, how do you propose we go about fixing it? It has tripped me up when I used it, so is there a way to distinguish whether we triggered it, or if it's on in general? I suppose if we really want the 'control center feel' that we get on phones, it might be best to have it as it is now, as it just shows the state of the HRM and allows you to toggle...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is - we can either have a boolean to keep track if we triggered it, or we delve into Bangle internals to find out, which is what the code does atm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotya, let me look and see how it's done now to see if we can add that functionality in!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - the code had the checks in, you could pinch that:

const hrm = (Bangle as any)._PWR?.HRM as undefined | Array<string> ;
const off = !hrm || hrm.indexOf(id) === -1;

Comment on lines +314 to +317
if (result === "close") {
terminateUI();
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to have the callback terminate the UI itself, rather than spread the responsibility and allocate strings in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was due to ui being undefined when it's called, because it's defined later in scope, so this just passes it down to a place where it can actually terminate the UI first.

Copy link
Collaborator

@bobrippling bobrippling Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show me the code where that's the problem and the error output? These functions are all executed after they're all defined, so I don't see how it can be a scoping problem.

For it being a problem that ui isn't set (not a scope problem, just logic), then:

var result = ctrl.cb(...); // if ui was `undefined` here then

if(result == "close"){
  terminateUI(); // there's no code between there and here which will make it defined
  return;
}

Co-authored-by: Rob Pilling <robpilling@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants