-
Notifications
You must be signed in to change notification settings - Fork 0
Small Fixes - Fix Default Camera Image #28
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
base: master
Are you sure you want to change the base?
Conversation
app/config.py
Outdated
| Path("static/image.jpg"), | ||
| arena_usb_path / "Corner.jpg", | ||
| Path('usercode/editable/team_logo.jpg'), | ||
| Path('/home/pi/game_logo.jpg'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Teddy and game logo are the same thing
| teamname_file: Path = Path("/home/pi/teamname.txt") | ||
| zone: bool = False | ||
|
|
||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to follow PEP8 (the python style guide in this). We could and should check this with CI but don't yet: https://peps.python.org/pep-0008/#block-comments
From PEP8 multiline blockcomments in python
are indented to the same level as that code. Each line of a block comment starts with a # and a single space (unless it is indented text inside the comment).
Paragraphs inside a block comment are separated by a line containing a single #.
I think the logic of this is that declaring strings in the middle of your code can sometimes do things see __doc__ in https://docs.python.org/3/reference/datamodel.html#the-standard-type-hierarchy
We really should do this in CI shouldn't we
| return | ||
|
|
||
| def _get_team_specifics(self): | ||
| def _get_team_image(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a better name than _get_team_specifics maybe _get_team_image_path would be an even better name?
|
|
||
| Only makes sense to run this if we are on a brain | ||
| Teamname is set per brain before shipping and allows unique graphics | ||
| for ID'ing teams in the arena. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this doc string is no longer 100% accurate?
|
|
||
| teamname_jpg = self.teamname_file.read_text().strip() + '.jpg' | ||
|
|
||
| self.camera_image_files[1] = Path(teamname_jpg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To a person reading only this function 1 is a bit of a magic number. Maybe a comment here to explain where this came from would be useful?
| """ | ||
| os.remove(cameraFile) | ||
| response = client.get("/files/image.jpg") | ||
| assert response.status_code == 200 No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to spin sheep out into seperate repo and commit changes
| os.remove(stimulus_path) | ||
|
|
||
| def test_get_image(client): | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra new line not needed by PEP8
Change website to get image from path /file/image.jpg (moved from /static/image.jpg). The image returned will be the first one available in the following order: