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

Multiple fixes and improvements #380

Merged
merged 67 commits into from
Mar 17, 2024
Merged

Conversation

matheusmoreira
Copy link
Collaborator

@matheusmoreira matheusmoreira commented Mar 12, 2024

madhogs and others added 30 commits October 2, 2023 17:23
This was causing a crash in the simulator when setting the location.

Fixes: joeycastillo#198
the sleep mode doesn't get set immediately, and needs to be waited upon.
* Introduce shell module for basic serial shell with argument parsing
* Introduce shell_cmd_list module for basic compile-time command
  registration
* Harden USB handling to hang less and drop fewer inputs
  - Service tud_task() with periodic TC0 timer interrupt
  - Service cdc_task() with periodic TC1 timer interrupt
  - Handle shell servicing in main app loop
  - Add a circular buffering layer for reads/writes
* Change newline prints to also send carriage return
* Refactor filesystem commands for shell subsystem
* Introduce new shell commands:
  - 'help' command
  - 'flash' command to reset into bootloader
  - 'stress' command to stress CDC writes

Testing:
* Shell validated on Sensor Watch Blue w/ Linux host
* Shell validated in emscripten emulator
* Tuned by spamming inputs during `stress` cmd until stack didn't crash
Aggregates all the data necessary for TOTP generation.
Generates a compound initializer for the given TOTP parameters.
Lessens repetition and allows functional definitions of TOTP records.
The data definitions are much shorter and easier to read now.
Computes the size of the array of TOTP records.
The compiler will likely evaluate it at compile time.
Selects the appropriate TOTP data structure
given the TOTP watch face state.
Using the new structured TOTP record data structure
allows the TOTP watch face to statically and implicitly
compute the total number of defined TOTP records.

Users can now simply add new keys and records in the designated area
and the watch face will compile and automatically use them with no need
to maintain a separate array size variable. Less chance of mistakes.
The TOTP watch face now keeps track of each key separately.
There is no need to compute offsets at runtime.
Update the copyrights to include full name attribution
to all who contributed to this watch face, including myself.

Also add an SPDX license identifier header comment to the files.

https://spdx.org/licenses/MIT.html
Implements an advanced pulsometer that can be recalibrated by the user.
The main clock face now displays the measured pulses per minute.
The day of month digits now display the pulsometer calibration.
The light button now cycles through integer graduations
which now range from 1 to 39 pulses per minute.
Long presses of the light button cycle by 10 instead of 1.

The watch face's responsiveness to input has been carefully optimized.
The code has been reorganized and generally improved.
Thoroughly document the new advanced pulsometer watch face
by describing what it is and how it works.
Update the copyrights to include full name attribution to all
who contributed to the pulsometer watch face, including myself.

Also add an SPDX license identifier header comment to the files.
Instances of the pulsometer state structure are only passed
to the pulsometer itself and only via the opaque context pointer.
No other code uses it. There is no need to expose it in a header file
so make it an implementation detail of the watch face.
It's not actually so simple and will only gain features from now on.
Just "clock face" also feels more canonical.
Instances of the clock state structure
are only passed to the clock face itself
and only via the opaque context pointer.
No other code uses it.

Thus there is no need to expose it in a header file.
So make it an implementation detail of the watch face
by localizing it inside the translation unit.
Sets or clears the specified indicator based on some boolean value.
matheusmoreira and others added 9 commits March 8, 2024 06:49
Fixes a crash due to use of uninitialized buffer when setting location.

Reported-by: eshrh <[email protected]>
Fixed-by: Wesley Aptekar-Cassels <[email protected]>
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#329
GitHub-Issue: joeycastillo#198
Fixes: joeycastillo#198
Implements the recommended workarounds for numerous silicon errata,
reducing power consumption and preventing freezes and hard faults.

Tested-by: Alex Maestas <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Alex Maestas <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Reviewed-by: Wesley Aptekar-Cassels <[email protected]>
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#340
GitHub-Related-Issue: joeycastillo#361
GitHub-Related-Issue: joeycastillo#359
Reference: https://ww1.microchip.com/downloads/aemDocuments/documents/MCU32/ProductDocuments/Errata/SAM-L22-Family-Silicon-Errata-and-Data-Sheet-Clarification-DS80000782.pdf
 - Change newline prints to also send carriage return
 - Introduce shell module for serial shell with argument parsing
 - Introduce shell command list for compile time command registration
 - Refactor file system commands for shell subsystem
 - Introduce new shell commands:
   - 'help' command
   - 'flash' command to reset into bootloader
   - 'stress' tests CDC serial writes of various lengths
     - optional delay parameter
 - Harden USB handling
   - Hangs less
   - Drops fewer inputs
 - Circular buffers for both reads and writes

Reported-by: Edward Shin <[email protected]>
Tested-by: Edward Shin <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Reviewed-by: James Haggerty <[email protected]>
Reviewed-by: Wesley Aptekar-Cassels <[email protected]>
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#344
Adds overridable C preprocessor definitions for every user preference.
Enables the user to set defaults and omit the preferences face.

The default behavior of the watch is preserved.

Suggested-by: Wesley Aptekar-Cassels <[email protected]>
Implemented-by: madhogs <[email protected]>
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#295
GitHub-Related-Issue: joeycastillo#291
Makes a long press of the ALARM button reset the face to current day.

Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-by: Wesley Aptekar-Cassels <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#316
Completely refactors the simple clock face
and lays the foundations for new features.

Also adds a compile time 24 hour mode only feature.

Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#373
Implements an advanced pulsometer that can be calibrated by the user.
Also features a streamlined and responsive user interface,
new documentation and generally improved code.

Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#371
Aggregates the TOTP credentials into a data structure,
making it easier to define and use the credentials.
Also incorporate backwards movement code from another branch.

Co-authored-by: Max Zettlmeißl <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#369
GitHub-Related-Pull-Request: joeycastillo#356
Currently, movement drops timeout events in case the previous loop
indicates that sleep is not possible. This is due to unintended
short circuiting behavior of && and is fixed with a temporary variable.

The static qualifier of can_sleep is also removed.

Helped-by: Alex Maestas <[email protected]>
Tested-by: Matheus Afonso Martins Moreira <[email protected]>
Tested-on-hardware-by: Matheus Afonso Martins Moreira <[email protected]>
Signed-off-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: joeycastillo#376
@theAlexes
Copy link
Collaborator

Thanks for the thorough commenting and testing :) Once #373 is updated on this branch, we'll be happy to merge this.

@matheusmoreira
Copy link
Collaborator Author

Something I forgot to mention about this branch: the clock face replaces the simple clock face in it. I thought it was alright since I've been running it for weeks now with no issues, and I also tested alarms and hourly chimes too. I can split it up again if needed though.

@theAlexes
Copy link
Collaborator

@joeycastillo did request that the simple clock face remain in place, so i think restoring that will make this branch mergeable :)

@matheusmoreira
Copy link
Collaborator Author

Yes. More thorough testing of the new clock face was required before it could replace the original, so I created this branch with the clock face replaced in order to flash it to my device for testing. Eventually I started merging all the other PRs as well and it turned into a Linux style next branch.

Not sure how much testing is enough to be honest. Been running it every day for weeks now, tested every function and have found no issues. What do you think @joeycastillo?

@matheusmoreira
Copy link
Collaborator Author

I'm on mobile at the moment and don't have access to a computer right now. When I get home I'll split the faces up in order to unblock this, assuming Joey hasn't changed his mind.

@theAlexes
Copy link
Collaborator

the extensive testing and thorough annotation is quite appreciated, for sure. I think the goal behind having the simple clock face separate from the advanced one is that the simple face serves as a low-complexity example for others to build new faces from, but we'll have to defer to Joey on this.

@matheusmoreira
Copy link
Collaborator Author

Yeah, I have this propensity to making small functions. I personally find it easier to understand but others found to be "Lisp-like", a quality which I didn't even try to refute due to my love of lisp. In my mind the refactored code is easier to understand, but I'll respect it if others don't find it so.

Restore the original simple clock face as requested.
@theAlexes theAlexes merged commit 955ac94 into joeycastillo:main Mar 17, 2024
2 checks passed
This was referenced Mar 17, 2024
@matheusmoreira matheusmoreira deleted the advanced branch March 17, 2024 03:23
@814d3
Copy link

814d3 commented May 12, 2024

@matheusmoreira, I checked simple clock face and "your" clock face in the online firmware builder. They act different in indicating alarm and hourly chime. Refering to #373 (comment) I thought they should be identically?

@matheusmoreira
Copy link
Collaborator Author

@814d3

They act different in indicating alarm and hourly chime.

Yes. It appears the hourly chime and alarm indicators had been swapped in movement. I noticed this and swapped them back. So clock_face differs from simple_clock_face but matches the stock watch's behavior. This appears to have been the original intention of the maintainers.

@814d3
Copy link

814d3 commented May 13, 2024

Well, what I read about the original watch behavior is, that the bell indicates a hourly signal und the waves a alarm - it's the same for the simple_clock_face, but not in clock_face. But you are right, in the documentation (#373 (comment)) it's swapped and should be corrected.

@matheusmoreira
Copy link
Collaborator Author

@814d3

You're absolutely right about that. I based the decision to switch the icons on the watch library documentation. Turns out the documentation was swapped all along!!

/// An enum listing the icons and indicators available on the watch.
typedef enum WatchIndicatorSegment {
WATCH_INDICATOR_SIGNAL = 0, ///< The hourly signal indicator; also useful for indicating that sensors are on.
WATCH_INDICATOR_BELL, ///< The small bell indicating that an alarm is set.
WATCH_INDICATOR_PM, ///< The PM indicator, indicating that a time is in the afternoon.
WATCH_INDICATOR_24H, ///< The 24H indicator, indicating that the watch is in a 24-hour mode.
WATCH_INDICATOR_LAP ///< The LAP indicator; the F-91W uses this in its stopwatch UI.
} WatchIndicatorSegment;

Screen elements descriptions, Module No. 593 manual

qw593.pdf

There's the alarm-on-mark and the time-signal-on-mark. The watch library documentation says the bell indicates alarms, but on the original module the bell indicates the time signal.

So this should definitely be fixed. Perhaps the fix should happen inside movement at the API level. The constants WATCH_INDICATOR_BELL and WATCH_INDICATOR_SIGNAL refer to the same thing.

matheusmoreira added a commit that referenced this pull request Aug 29, 2024
I misinterpreted the documentation and swapped the alarm and time signal
indicators in the clock face. This patch brings it back in sync with
the original watch's behavior as described in the manual.

Reported-by: 814d3 (GitHub)
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: #433
References: #380
References: #132
matheusmoreira added a commit that referenced this pull request Aug 30, 2024
I misinterpreted the documentation and swapped the alarm and time signal
indicators in the clock face. This patch brings it back in sync with
the original watch's behavior as described in the manual.

Reported-by: 814d3 (GitHub)
Reviewed-by: Matheus Afonso Martins Moreira <[email protected]>
GitHub-Pull-Request: #433
References: #380
References: #132
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.

7 participants