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

Fix GHST telemetry #6462

Merged
merged 1 commit into from
Dec 31, 2020
Merged

Fix GHST telemetry #6462

merged 1 commit into from
Dec 31, 2020

Conversation

DzikuVx
Copy link
Member

@DzikuVx DzikuVx commented Dec 30, 2020

Fixes #6457

After overflow, telemetry was generated every 2ms instead of 100ms. That was slowly clogging serial port so failsafe was triggered.

@tonycake you might be interested in this one

@DzikuVx DzikuVx added this to the 2.7 milestone Dec 30, 2020
@DzikuVx
Copy link
Member Author

DzikuVx commented Dec 30, 2020

@Christoph-Hofmann you might want to take a look

@Christoph-Hofmann
Copy link

I checked your changes. It work now fine.
Now flight times with ghost longer than 4300s are possible 👍
@DzikuVx Pawel you are my hero :-)

@@ -163,7 +163,7 @@ bool checkGhstTelemetryState(void)
// Called periodically by the scheduler
void handleGhstTelemetry(timeUs_t currentTimeUs)
{
static uint32_t ghstLastCycleTime;
static timeUs_t ghstLastCycleTime;
Copy link
Member

Choose a reason for hiding this comment

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

32-bit microtime overflow, ta-daaah!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice Find! thanks to everyone involved in finding this, looks like it wasn't easy to find.
So.. looking at the BF driver, I guess the behavior is different because timeUs_t is 32 bits in BF?
At least I can't find anywhere that defines USE_64BIT_TIME.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, AFAIK BF is 32-bit only, therefore even if uint32_t overflows, this doesn't mess with scheduling because uint32 - uint32 will always yield a correct result even if overflow happens.

Here it's different, after crossing the 2^32 boundary uint64 - uint32 will always yield a result larger than 4294 seconds which causes the telemetry driver to think "Oh shit, I sent the last telemetry packet over 1 hour ago" and schedule the new one immediately.

@digitalentity digitalentity merged commit b40eaf4 into master Dec 31, 2020
@digitalentity digitalentity deleted the dzikuvx-fix-ghst-telemetry branch December 31, 2020 07:20
@DzikuVx DzikuVx mentioned this pull request Feb 8, 2021
@DzikuVx DzikuVx modified the milestones: 2.7, 2.6.1 Feb 23, 2021
DzikuVx pushed a commit that referenced this pull request Feb 23, 2021
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.

ghost protocol failsafe after ca. 4300s ontime
4 participants