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

✨ 📶 Add wireless V5 uploading #47

Merged
merged 7 commits into from
Feb 17, 2019
Merged

✨ 📶 Add wireless V5 uploading #47

merged 7 commits into from
Feb 17, 2019

Conversation

edjubuh
Copy link
Member

@edjubuh edjubuh commented Feb 1, 2019

Summary:

Adds support for wireless uploading. Specifically

  • Enables joystick port detection
  • Negotiation to dedicated download channel
  • Currently we just check if the connected to a controller, and always negotiate to download channel. We could be able to do a better job and only ask for transfer when we know we're communicating with the Brain over VEXnet and not on download channel. I'm not currently aware of how to query this info so it will have to wait
  • Improved how timeout gets assigned when invoking commands. Now have the ability to change the default timeout instead of just letting default parameters fall through. This is useful since we need to increase the timeout of everything when communicating wirelessly.

Motivation:

Everyone and their mother wants this

Impact:

Wireless upload is slow and PROS binaries are relatively large considering the bandwidth when uploading wirelessly.

#46, purduesigbots/pros#89 all aim to address this

References (optional):

Test Plan:

  • Upload kernel project wirelessly (albeit slowly)
  • Upload kernel project over the wire
  • Upload hot /cold Forkner wirelessly
  • Upload hot /cold Rogers wirelessly
  • Upload a project via V5 Controller through smart port
  • prosv5 lsusb shows controller as a system port
  • Large files get a prompt to make sure you want to upload

Summary: tba

Test Plan: tba

Reviewers: O3 The Q Continuum!

Tags: #zorp

Differential Revision: https://phabricator.purduesigbots.com/D275
@edjubuh edjubuh added feature A brand new feature in progress This is currently being worked on p: normal Normal priority labels Feb 1, 2019
@edjubuh edjubuh added this to the 3.1.4 milestone Feb 1, 2019
@edjubuh edjubuh self-assigned this Feb 1, 2019
@HotelCalifornia HotelCalifornia mentioned this pull request Feb 2, 2019
4 tasks
Copy link
Contributor

@HotelCalifornia HotelCalifornia left a comment

Choose a reason for hiding this comment

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

As far as the actual changes made in this diff, everything looks pretty good. I just had two nits about adjacent code that might be cleared up while we're in the neighborhood.

Also, approval is contingent on the rest of the test plan being executed.

pros/serial/devices/vex/v5_device.py Outdated Show resolved Hide resolved
pros/serial/devices/vex/vex_device.py Show resolved Hide resolved
@Razat101
Copy link

Razat101 commented Feb 2, 2019

I successfully uploaded code with smart cable connection.

@edjubuh

This comment has been minimized.

@edjubuh edjubuh added in testing This is currently in testing and removed in progress This is currently being worked on labels Feb 5, 2019
HotelCalifornia added a commit that referenced this pull request Feb 12, 2019
#### Summary:
Adds a new option for uploading binaries to the V5:

```
$ prosv5 u --compress-bin
```

The default value is true (and can be switched off with `--no-compress-bin`). When compression is enabled, .bin files to be uploaded are first compressed with gzip, which is supported natively on the V5.

(there is a side change included with this PR that adds a missing letter to the name of an internally used class)

#### Motivation:
One of a series of PRs aimed at improving speed and reliability of wireless uploading.

##### References (optional):
<!-- If this PR is related to an issue or task, reference it here (e.g. closes #1) -->
Together with #46, this blocks #47 

#### Test Plan:
- [x]  run prosv5 u (targeting a v5). bin/output.compressed.bin should be created, and it should have uploaded successfully
- [x] run prosv5 u --no-compress-bin (targeting a v5). only the normal bin/output.bin should be uploaded
- [x] run prosv5 u (targeting a cortex). nothing out of the ordinary should happen. Tested both with explicit `prosv5 u --compress-bin` and `prosv5 u --no-compress-bin`
- [x] Test compression checkbox in interactive upload modal

#### Commits
* Add binary compression before uploading on V5

* Remove unused import

* don't compress ini files when uploading

* �� address review concerns

* add interactive UI option for compression
Copy link
Contributor

@HotelCalifornia HotelCalifornia left a comment

Choose a reason for hiding this comment

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

Let's do it

@edjubuh edjubuh merged commit 284c85c into develop Feb 17, 2019
@edjubuh edjubuh mentioned this pull request Feb 18, 2019
2 tasks
@kunwarsahni01 kunwarsahni01 deleted the wireless-uploading branch October 29, 2021 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A brand new feature in testing This is currently in testing p: normal Normal priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants