Skip to content

Collaboration on https://github.com/meshtastic/firmware/pull/4043 #1

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

Merged
slash-bit merged 2 commits into from
Jun 12, 2024
Merged

Collaboration on https://github.com/meshtastic/firmware/pull/4043 #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f3db83
Hold position, even if number of frames increases
todd-herbert Jun 12, 2024
64d8cdf
Hold position, if handling an outgoing text message
todd-herbert Jun 12, 2024
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
39 changes: 28 additions & 11 deletions src/graphics/Screen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2054,11 +2054,12 @@ void Screen::setScreensaverFrames(FrameCallback einkScreensaver)
#endif

// restore our regular frame list
void Screen::setFrames(bool holdPosition = false)
void Screen::setFrames(bool holdPosition)
{
uint8_t currentFrameNum = ui->getUiState()->currentFrame;
LOG_DEBUG("Showing standard frame number %d\n", currentFrameNum);

// Which frame we are currently showing. We might want to return here after the new frames are set
uint8_t oldFrame = ui->getUiState()->currentFrame;

LOG_DEBUG("showing standard frames\n");
showingNormalScreen = true;

#ifdef USE_EINK
Expand Down Expand Up @@ -2150,13 +2151,23 @@ void Screen::setFrames(bool holdPosition = false)

setFastFramerate(); // Draw ASAP

// In some situations, we'd like to return to the same frame
static size_t oldNumFrames = numframes;

@slash-bit slash-bit Jun 12, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I am not sure of assigning oldNumFrames = numframes here. As we have already reset the value of numframes to 0 in row 2085, calculated new value in 2093 ?
I think we just declare oldNumFrames. And assign value of numframes at the end in row 2169 , to use it next time ?

@todd-herbert todd-herbert Jun 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Actually, I do see what you're getting at there.

Just to be clear: because it's static, it will only be declared and initialized the very first time that method is called. Every other time, it will preserve the value from the end of the previous call.

The very first time that method is called though, you're right, it will be initialized and then set again with the same value. In theory, the first setFrames call should always be setFrames(false), so it would probably be okay to declare it with no initial value, and wait for it to be set at the end of that first call.

That being said, it is nice that it's being initialized before that if(holdPosition) block, just in case the very first call to setFrames is somehow setFrames(true). There's probably no reason it couldn't be initialised with some fixed value like 2 instead. In theory that initial value will never be used; it'd just be insurance against some unexpected weirdness.

Honestly, I didn't put this much thought into it when I initially wrote that line. I think I probably moved it from somewhere else without realizing that it's a bit redundant now.

If you wanted to be really diligent, you could even initialize it as 0, and then assert(oldNumFrames) at the start of that if(holdPosition) block, to throw an error if it does somehow get used before it's meant to, although that might be overkill.

Whatever suits your sense of design!

if (holdPosition) {
ui->switchToFrame(currentFrameNum); // Attempt to return to same frame after rebuilding the frames,
// if holdPosition is true (currently only Screen::handleStatusUpdate calls this
} else {
continue; // We leave the displayed frame as it is or chnage focuse to new frame
}
}
if (oldFrame == (oldNumFrames - 1)) // If we were on the final frame (settings)
ui->switchToFrame(numframes - 1); // then move back to the final frame

else if (oldFrame == (oldNumFrames - 2)) // If we were on the log buffer frame
ui->switchToFrame(numframes - 2); // then move back there

else if (oldFrame > numframes - 1) // If we were on a frame that no longer exists
ui->switchToFrame(0); // back to the first frame

else
ui->switchToFrame(oldFrame); // Otherwise, go back to the same frame
}
oldNumFrames = numframes; // Store how many frames we have, in case we want to "restore position" next time
}

@slash-bit slash-bit Jun 12, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

That logic is clear now .
Will flash all to my T-Echo tonight and test.
(b^_^)b


void Screen::handleStartBluetoothPinScreen(uint32_t pin)
{
Expand Down Expand Up @@ -2667,7 +2678,13 @@ int Screen::handleStatusUpdate(const meshtastic::Status *arg)
int Screen::handleTextMessage(const meshtastic_MeshPacket *packet)
{
if (showingNormalScreen) {
setFrames(); // Regen the list of screens (will show new text message)
// Outgoing message
if (packet->from == 0)
setFrames(true); // Return to same frame

// Incoming message
else
setFrames(false); // Regen the list of screens (will show new text message)
}

return 0;
Expand Down
2 changes: 1 addition & 1 deletion src/graphics/Screen.h
View file
Open in desktop

@slash-bit slash-bit Jun 12, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

That's perfect , thanks.

Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ class Screen : public concurrency::OSThread
void handleShutdownScreen();
void handleRebootScreen();
/// Rebuilds our list of frames (screens) to default ones.
void setFrames();
void setFrames(bool holdPosition = false);

/// Try to start drawing ASAP
void setFastFramerate();
Expand Down