Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for new devices and update code #1

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

janbina
Copy link

@janbina janbina commented Feb 29, 2024

Hi, just bought new watch and this watchface wasn't available for them...
Had to make some changes for it to compile with the latest sdk (6.4.2-2024-01-04).
Then made it available for new models of already supported devices.
Only tested on my Forerunner 265, works fine.

@docwilco
Copy link
Collaborator

You've misrepresented your PR some, this is much more than just supporting new devices.

Can you split things out a bit?

Also, can you explain why you've made wide sweeping changes to the code? I don't understand, for instance, why you changed the type of hash_marks_width.

The "fix imports" commit is also quite baffling as to why you would want to do that. And honestly, changing things like Gfx to Graphics is almost insulting, because it's absolutely disrespecting the author's preferences.

@docwilco
Copy link
Collaborator

Oh, and changing line endings? No.

@docwilco docwilco self-assigned this Feb 29, 2024
@docwilco
Copy link
Collaborator

Oh, and removing types from local vars? You'll have to have a really good reason for me to accepts that.

@janbina
Copy link
Author

janbina commented Feb 29, 2024

ahh, feel free to just close it, I didn't mean to insult anyone :)
I really just wanted to make it compile and run on my new watch, which it does now, so I'm happy. I just though that the pr might help someone finding himself in the same situation...

removing types from local vars? You'll have to have a really good reason for me to accepts that

the latest version of compiler says this is an error

ERROR: SpotlightView.mc:193: Invalid explicit typing of a local variable. Local variable types are inferred.

why you changed the type of hash_marks_width

similarly, assigning float array to Array<Number> raised a compilation error here:

var hash_marks_width as Array<Number> = new Float[NUM_HASH_MARKS]

looking at it now, changing the Float to Number instead might have been better? but I'm not sure, I don't know the code, just wanted to make the error go away :)

The "fix imports" commit is also quite baffling as to why you would want to do that.

again, there was probably some change in the compiler, so when you use using Toybox.Lang;, there is an error ERROR: SpotlightView.mc:10: Cannot resolve type 'Number'. which goes away if you just use import instead of using. To be honest, I didn't investigate why does it behave like this, so there might be a different/better solution. Again, I only made the change for it to compile, I didn't mean to question the codestyle of the original author or insult him.

peace

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.

2 participants