QoL Improvements - #2
Merged
Merged
Conversation
Added vertical UI Added multi-collar support/grouping Added collar naming
Fixed light transmission which would transmit upon changing from ON/OFF Fixed font/bar size in horizontal light view
Member
|
build fails |
Contributor
Author
|
haha yep forgot uFBT exists, i build via FBT itself in the firmware which was the issue, should be good now i hope since im on a CFW! |
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenShock Flipper app with UI/UX improvements and expanded device management, including a new transmit UI mode, persistent screen settings, and multi-shocker sync grouping support with editable collar naming.
Changes:
- Added a Settings screen and persisted “Transmit UI: Vertical/Horizontal” preference to a config file.
- Reworked shocker persistence to support editable “stem” names, sync-group storage, and sorted listing.
- Implemented multi-shocker transmit grouping (concatenated packets) and a new vertical transmit UI.
Reviewed changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| storage.h | Extends saved shocker schema with sync group and introduces new save/write/lookup APIs. |
| storage.c | Implements unique name generation, write/rename-delete behavior, sync-group field persistence, and sorted listing. |
| settings.h | Declares settings load/save API for persisted UI preference. |
| settings.c | Implements settings persistence via FlipperFormat config file. |
| protocols.h | Increases max pulse buffer size to support concatenated multi-packet TX. |
| openshock.c | Major UI/state refactor: ViewDispatcher + TextInput, settings screen, vertical transmit UI, sync-group TX, and naming flow. |
Comments suppressed due to low confidence (1)
openshock.c:1578
- Same issue in the vertical transmit handler:
s->transmittingis set true even when pulse generation fails, leaving the app in a "transmitting" state without an active TX to stop on OK release.
s->transmitting = true;
OokPulse pulses[OPENSHOCK_MAX_PULSES];
size_t count = 0;
if(tx_fill_pulses(s, pulses, &count) && count > 0) {
openshock_tx_start(tx, pulses, count);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11
to
+13
| bool openshock_shocker_unique_stem(ShockerModel model, uint16_t id, char* stem_out, size_t stem_sz) { | ||
| Storage* storage = furi_record_open(RECORD_STORAGE); | ||
| storage_simply_mkdir(storage, OPENSHOCK_SAVE_DIR); |
Comment on lines
+29
to
+35
| FileInfo info; | ||
| FS_Error err = storage_common_stat(storage, path, &info); | ||
| if(err != FSE_OK) { | ||
| snprintf(stem_out, stem_sz, "%s", candidate); | ||
| ok = true; | ||
| break; | ||
| } |
Comment on lines
+13
to
+15
| void openshock_settings_load(bool* transmit_vertical_ui_out) { | ||
| *transmit_vertical_ui_out = false; | ||
|
|
Contributor
There was a problem hiding this comment.
@Play2BReal could we make this pass by reference instead of a pointer?
Comment on lines
+1197
to
+1203
| if(s->screen == ScreenReceive) { | ||
| DecodedShocker d; | ||
| if(openshock_rx_get_result(app->rx, &d)) { | ||
| s->rx_result = d; | ||
| s->rx_found = true; | ||
| } | ||
| } |
Comment on lines
1457
to
1461
| s->transmitting = true; | ||
| OokPulse pulses[OPENSHOCK_MAX_PULSES]; | ||
| size_t count = openshock_encode( | ||
| c->model, | ||
| c->shocker_id, | ||
| c->command, | ||
| c->intensity, | ||
| c->channel, | ||
| pulses, | ||
| OPENSHOCK_MAX_PULSES); | ||
| if(count > 0) { | ||
| size_t count = 0; | ||
| if(tx_fill_pulses(s, pulses, &count) && count > 0) { | ||
| openshock_tx_start(tx, pulses, count); |
Contributor
|
Yeah probably look at some of that, im still making my way trough the codereview |
Contributor
|
Nah its fine, we can merge this and then fix the small nits after :) |
Contributor
Author
|
haha awesome thank you!!, actually gave a buddy my 2nd collar so i'll have to grab a 2nd one later on to test! |
hhvrc
added a commit
that referenced
this pull request
Jul 28, 2026
- Fix clang-format lint failures introduced in the ViewDispatcher migration (no logic changes, formatting only). - Restore periodic redraw via a FuriTimer so the receive screen updates live again; the old ~50ms poll loop was dropped when the app moved to ViewDispatcher and nothing replaced it. - Only set s->transmitting after pulse generation succeeds, so a failed tx_fill_pulses (e.g. a sync group too large for OPENSHOCK_MAX_PULSES) can't leave the flag stuck true. - openshock_shocker_unique_stem: validate stem_out/stem_sz, and only treat FSE_NOT_EXIST as "name is free" instead of any stat error. - openshock_settings_load: return the bool by value instead of writing through an out-pointer, removing the NULL-deref surface Copilot flagged and addressing hhvrc's PR comment about avoiding a raw pointer here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added vertical screen
Added screen settings
Added multishocker support + grouping
Added shocker naming
Tested
CaiXianlin
Untested
Petrainer
Wellturn
(note still new to PR's so go easy on me haha)