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.

Sign up for GitHub

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

implementing #1594 #1616

Merged
merged 18 commits into from
Apr 1, 2023
Merged

implementing #1594 #1616

Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d88eed4
Update ble_handlers.cpp
discip Mar 11, 2023
6174d80
Update ble_handlers.cpp
discip Mar 11, 2023
e12903e
Update ble_handlers.cpp
discip Mar 11, 2023
1d47a85
Merge branch 'dev' into discip-patch-1
discip Mar 11, 2023
2319eeb
Update ble_handlers.cpp
discip Mar 13, 2023
3e3d0bb
Merge branch 'dev' into discip-patch-1
discip Mar 13, 2023
36d5c05
Merge branch 'dev' into discip-patch-1
discip Mar 14, 2023
656f1a3
Merge branch 'dev' into discip-patch-1
discip Mar 14, 2023
c7bdbbb
Merge branch 'dev' into discip-patch-1
discip Mar 14, 2023
60f0437
Merge branch 'dev' into discip-patch-1
discip Mar 15, 2023
bed400e
Merge branch 'dev' into discip-patch-1
discip Mar 17, 2023
d8b6fac
Merge branch 'dev' into discip-patch-1
discip Mar 20, 2023
744dd6b
Merge branch 'dev' into discip-patch-1
discip Mar 25, 2023
3a4638e
Merge branch 'dev' into discip-patch-1
discip Mar 26, 2023
fe9043f
Merge branch 'dev' into discip-patch-1
discip Mar 26, 2023
45c00dc
Merge branch 'dev' into discip-patch-1
discip Mar 31, 2023
f6975b0
Update ble_handlers.cpp
Ralim Apr 1, 2023
0bbf8fb
Merge branch 'dev' into discip-patch-1
Ralim Apr 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions source/Core/BSP/Pinecilv2/ble_handlers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,19 @@ int ble_char_write_setting_value_callback(struct bt_conn *conn, const struct bt_
}
} else if (uuid_value < SettingsOptions::SettingsOptionsLength) {
setSettingValue((SettingsOptions)(uuid_value), new_value);
// @TODO refactor to make this more usable
if (uuid_value == SettingsOptions::OLEDInversion) {
switch (uuid_value) {
case SettingsOptions::OLEDInversion:
OLED::setInverseDisplay(getSettingValue(SettingsOptions::OLEDInversion));
}
if (uuid_value == SettingsOptions::OLEDBrightness){
break;
case SettingsOptions::OLEDBrightness:
OLED::setBrightness(getSettingValue(SettingsOptions::OLEDBrightness));
}
if (uuid_value == SettingsOptions::OrientationMode){
break;
case SettingsOptions::OrientationMode:
OLED::setRotation(getSettingValue(SettingsOptions::OrientationMode) & 1);
break;
default:
BT_ASSERT(uuid_value == uuid_value);
Copy link
Owner

Choose a reason for hiding this comment

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

Why this assert?

Copy link
Contributor

Choose a reason for hiding this comment

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

the intention was to notify that this code path was reached. In general it should mean that this block needs to be revisited (or something went really wrong) - since these are currently the only parameters which require additional handling to apply them without restart.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure why this would be used; this will always evaluate to true and thus not crash the chip.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't disagree. should we crash the chip?

we left it this way to ask you what should be done, basically.

Copy link
Owner

Choose a reason for hiding this comment

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

I think for now we remove this; but I'm going to have to cleanup an "apply" function for settings

break;
}
return len;
}
Expand Down