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

bootloader dialog Release flashing not picking new release #576

Merged
merged 4 commits into from
Jan 27, 2022

Conversation

knmcguire
Copy link
Contributor

If the CFclient is open and and one wants to first flash one version of a release firmware and then another in the same session, it will still flash the first version because it will first check if a file (any file, doesn't matter which version) is already downloaded. These release will get renamed so it will not be able to distinguish which version of the release it is.

The only problem is that this will be more vulnerable to internet loss while pressing program, as the dialog in such a situation might freeze. Removing the check will cause the dialog to make more inquires to the release, and currently there is no signal within Firmware downloader that indicates a fail.

@jonasdn
Copy link
Contributor

jonasdn commented Jan 26, 2022

Is there no way to receive the checksum of the file to download, from headers or otherwise? Then we could compare that checksum (md5 or otherwise) with the downloaded one to determine if they are the same.

Hmm checking:
https://api.github.com/repos/bitcraze/crazyflie-release/releases

There is a size field. That could be used perhaps? As heuristic, if size differs it cannot be the same file.

It should already be parsed in bootloader.py, right?

@knmcguire
Copy link
Contributor Author

Yes true, I'll see if I can do something for this.

Still in the event of an closed internet connection right after the cfclient open, this freezing will still occur, but perhaps it won't happen often. The question is also if we want to account for that corner case and make a separate thread signal for that.

@krichardsson
Copy link
Contributor

Could we not name the file based on the version that was downloaded instead?

@krichardsson
Copy link
Contributor

Something like this:

    def _download_release(self, signal, release_name, url):
        """ Downloads the given release and calls the callback signal
            if successful.
        """
        filepath = os.path.join(self._tempDirectory.name, release_name.split(' ')[-1])
        try:
            # Check if we have an old file saved and if so, ensure it's a valid
            # zipfile and then call signal
            with open(filepath, 'rb') as f:
                previous_release = zipfile.ZipFile(f)
                # testzip returns None if it's OK.
                if previous_release.testzip() is None:
                    logger.info('Using same firmware-release file at'
                                '%s' % filepath)
                    signal.emit(release_name, filepath)
                    return
        except FileNotFoundError:
            try:
                # Fetch the file with a new web request and save it to
                # a temporary file.
                with urlopen(url) as response:
                    with open(filepath, 'wb') as release_file:
                        release_file.write(response.read())
                    logger.info('Created temporary firmware-release file at'
                                '%s' % filepath)
                    signal.emit(release_name, filepath)
            except URLError:
                logger.warning('Failed to make web request to get requested'
                               ' firmware-release')

Also remove self._filepath in init()

@knmcguire
Copy link
Contributor Author

thanks @krichardsson ! that is indeed a good one. I've added it, tried it out with first flashing the candidate release, then flashing 2021.06 and then back to the candidate release again (as it would handle all the try exception cases we are implementing now) and that all worked out :)

if this looks good now, let's merge it?

@krichardsson krichardsson merged commit bd904fa into master Jan 27, 2022
@krichardsson krichardsson deleted the bootloader-dialog-fix branch January 27, 2022 14:42
@krichardsson krichardsson added this to the 2022.01 milestone Jan 27, 2022
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.

3 participants