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

Small Fixes - Fix Default Camera Image #28

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ poetry shell

Start the server in development mode (hot reload):
```
poetry run uvicorn app:app --reload
poetry run uvicorn app:shepherd --reload
```

To deploy run (chosen port and ip are optional):
Expand Down
45 changes: 22 additions & 23 deletions app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,30 @@ class Settings:
teamname_file: Path = Path("/home/pi/teamname.txt")
zone: bool = False

"""
Copy link
Member

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

Pick a start image in order of preference :
1) Camera Image (Deleted on Start)
2) Corner Image (USB)
3) Team Image (Uploaded)
4) Generic Corner Image (Arena USB Exists)
5) Teddy Image
"""
camera_image_files: list[Path] = [
Path("static/image.jpg"),
Path("static/temp.jpg"),
arena_usb_path / "Corner.jpg",
Path('usercode/editable/team_logo.jpg'),
Path("static/game/teddy.jpg")
]

# tempfile.mktemp is deprecated, but there's no possibility of a race --
usr_fifo_path = tempfile.mktemp(prefix="shepherd-fifo-")

def __init__(self):
self._on_brain()
self._init_usercode_folder()
self._zone_from_USB()
self._get_team_image()

# os.mkfifo raises if its path already exists.
os.mkfifo(self.usr_fifo_path)
Expand Down Expand Up @@ -72,35 +89,17 @@ def _zone_from_USB(self):
self.zone = str(i)
return

def _get_team_specifics(self):
def _get_team_image(self):
Copy link
Member

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?

"""Find information set on each brain about the team

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.
Copy link
Member

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?


Pick a start image in order of preference :
1) We have a team corner image on the USB
2) The team have uploaded their own image to the robot
3) We have a generic corner image on the USB
4) The game image
"""
teamname_jpg = 'none'
if self.teamname_file.exists():
teamname_jpg = self.teamname_file.read_text().replace('\n', '') + '.jpg'
else:
teamname_jpg = 'none'

start_img_path = self.arena_usb_path / teamname_jpg
if not start_img_path.exists():
start_img_path = Path('usercode/editable/team_logo.jpg')
if not start_img_path.exists():
start_img_path = self.arena_usb_path / 'Corner.jpg'
if not start_img_path.exists():
start_img_path = Path('/home/pi/game_logo.jpg')

if start_img_path.exists():
displayed_img_path = Path('static/image.jpg')
displayed_img_path.write_bytes(start_img_path.read_bytes())

teamname_jpg = self.teamname_file.read_text().strip() + '.jpg'

self.camera_image_files[1] = Path(teamname_jpg)
Copy link
Member

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?


config = Settings()
10 changes: 9 additions & 1 deletion app/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
import logging

from fastapi import APIRouter, HTTPException, UploadFile, File, Request
from fastapi.responses import FileResponse
from pydantic import BaseModel

from app.run import States
from app.run import runner
import app.editor
import app.upload

from app.config import config

# ==============================================================================
# Runner router
Expand Down Expand Up @@ -87,3 +88,10 @@ async def save_file(filename: str, request: Request):
def delete_file(filename: str):
app.editor.delete_file(filename)


@files_router.get('/image.jpg')
def get_image():
for camera_file in config.camera_image_files:
if camera_file.exists():
return FileResponse(camera_file)
raise HTTPException(status_code=404, detail="Item not found")
2 changes: 1 addition & 1 deletion static/editor/bundle.js

Large diffs are not rendered by default.

Binary file added static/game/teddy.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions test/test_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,23 @@ def test_create_and_delete(client):
finally:
if stimulus_path.exists():
os.remove(stimulus_path)

def test_get_image(client):

Copy link
Member

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

"""
Test Camera Image
"""
cameraFile = config.camera_image_files[0]
if not cameraFile.exists():
cameraFile.write_text("Hello World!")

response = client.get("/files/image.jpg")
assert response.status_code == 200


"""
Test No Camera Image
"""
os.remove(cameraFile)
response = client.get("/files/image.jpg")
assert response.status_code == 200
Copy link
Member

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