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 No Camera bug, Port to Python3, Update to Sugargame v1.3 #11

Merged
merged 15 commits into from
Mar 22, 2024

Conversation

srevinsaju
Copy link
Member

@srevinsaju srevinsaju commented Jan 16, 2020

Tested Coverage - 98%

Known Issues:
The pygame uses hardcoded screen sizes. However, it would be working on all screens above 800x600 which was already an issue which existed on the master branch too. See #12

Self-reviewed according to Review Guidelines. All tests passed

@srevinsaju
Copy link
Member Author

Make toolbar buttons insensitive when camera isn't found

Signed-off-by: Ibiam Chihurumnaya <[email protected]>
@chimosky
Copy link
Member

chimosky commented Jan 20, 2020

Tested, made some changes and sent a PR to your repo.
Changes made

  • Change except clause that starts camera to if statement as except clause seemed to have no explanation when there was one.
  • Make toolbar buttons insensitive when no camera is detected.

@quozl
Copy link
Contributor

quozl commented Jan 22, 2020

Tested with a camera. The camera live image is on display, but the buttons are insensitive. I did not diagnose. Are you sure about not calling pygame.camera.init()? Are you sure about hfilp spelling?

@chimosky
Copy link
Member

Tested with a camera. The camera live image is on display, but the buttons are insensitive. I did not diagnose. Are you sure about not calling pygame.camera.init()? Are you sure about hfilp spelling?

pygame.camera.init() was called as camera.init(), you're right it should be hflip. Can you diagnose when you have the time?, as the issue would most likely be from the value ofself._has_camera.

@srevinsaju
Copy link
Member Author

@chimosky I suspected that, hfilp is my mistake, I hadn't tested your changes. I will make those changes if something gone wrong.

@srevinsaju
Copy link
Member Author

@chimosky fixed the typo on 6bf9c7a. Seems like sugargame waits for the init to get over and then starts the game. Maybe the has_camera has to be called within pygame file

@srevinsaju
Copy link
Member Author

@chimosky @quozl I have added another commit here, it works now for me 942ad61

@chimosky
Copy link
Member

@chimosky fixed the typo on 6bf9c7a. Seems like sugargame waits for the init to get over and then starts the game. Maybe the has_camera has to be called within pygame file

When self.game is initialized, run is called and when that happens self._has_camera will have it's value. has_camera doesn't need to be called within the pygame file - I assume you're talking about sugargame - as it's not used there.

@srevinsaju
Copy link
Member Author

srevinsaju commented Jan 22, 2020

@chimosky did you compare the test between f9a02f4 and 942ad61? We have self.game = pano.PanoCapture(self) entered, However, this sets self.game.Has_camera as false at the startup. This forces the build_toolbar to assume the camera is not there, so lets make the buttons inactive. But, later, when the run() is called, (in sugargame.canvas, we are only giving the reference to the self.game.run and not self.game.run(), thesugargame.canvas calls the run function only after build_toolbar is successfully executed (else the toolbar won;t be created)

@quozl
Copy link
Contributor

quozl commented Jan 22, 2020

Tested as at 942ad61;

  • on a native install to a laptop, the camera image is shown and controls work; though the stitching seems not to work very well, and there is a PyGIWarning for Gtk,
  • on a virtual machine with no camera, the controls are still sensitive (should be insensitive), the canvas is not drawn on, and there is no message about no camera, but there was a traceback;
Traceback (most recent call last):
  File "/usr/share/sugar/activities/Panorama.activity/pano.py", line 84, in run
    self.camera = camera.Camera(self.camlist[0], self.size, "RGB")
IndexError: list index out of range

@quozl
Copy link
Contributor

quozl commented Jan 22, 2020

Closes #9.

@srevinsaju
Copy link
Member Author

@quozl can you please check if you tested my branch on the VM? It seems like that error should not be created because it checks for !=0. If there is no camera, that block should not have been executed in the first place. Tested on Sugar Debian Live 0.116. No errors

@quozl
Copy link
Contributor

quozl commented Jan 24, 2020

I agree, it must have been a mistake. Tested 942ad6. Still the PyGIWarning for Gtk. Does say Camera not found, over on the right side of the display. Does consume all CPU cycles. Does mark buttons insensitive.

Could you fix the CPU issue? Add a pygame.clock.

@srevinsaju
Copy link
Member Author

@quozl the camera not found appearing too outside the screen is because the screen sizes were hardcoded to match the olpc infinity screen sizes. I will try to add the pygame.clock.

@quozl
Copy link
Contributor

quozl commented Jan 24, 2020

Thanks. I can't find where the screen size is coded to any OLPC product. Where is this in the code?

@srevinsaju
Copy link
Member Author

I mean the standard olpc screen resolution, I don't remember maybe 1280*768, I am not sure, previewing it on a higher resolution makes it look correct, we have to change the hardcoded screen resolution numbers to Gdk. Screen. width() /some value to make it look the same

@quozl
Copy link
Contributor

quozl commented Jan 24, 2020

OLPC XO was 1200x900 pixels. I've looked again and can't find anything that says that in this repository. My test VM is 1024x768 pixels.

@srevinsaju
Copy link
Member Author

Yes @quozl the panorama activity is designed to look the best on 1200x900 resolution. If you try running it on a higher resolution, there will be still extra spaces below. I will try my best to use Gdk.Screen.wifth() and height

@srevinsaju
Copy link
Member Author

@quozl fixed the CPU usage error on 57f80a7 and fixed the warnings on 424141e

@srevinsaju
Copy link
Member Author

... -> PR 😄

@drLite35 drLite35 mentioned this pull request Mar 22, 2024
@quozl quozl merged commit d0015d3 into sugarlabs:master Mar 22, 2024
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