PiSupply / PaPiRus

Resources for PaPiRus ePaper eInk displays
https://www.pi-supply.com/product/papirus-epaper-eink-screen-hat-for-raspberry-pi/
Other
347 stars 84 forks source link

Add stub of a test program, correct a few notes and some tidying up #38

Closed ukscone closed 6 years ago

ukscone commented 8 years ago

i've added the start of a test program and corrected a few notes etc.

shawaj commented 8 years ago

In setup.py do we need to have papirus-logo.png? and also, if so, it should be bin/papirus-logo.png same as others?

And in the README:

# Write text to the screen at selected point
# text.write(text, size, x,y)
+text.write("hello world", 20 10, 10 )

Should there not be a comma after 20? And what are changes in text.py about x and y axis for? Do we not need a definable size as well?

Other than that what is gitignore on .pyc for?

I think snakes and framepush code is done, just need to find out why they aren't here on git!

ukscone commented 8 years ago

sorry i think the missing comma was a typo that occurred when i was moving stuff from the dev directory to the repo dir as github was being a right PITA. you have the definable size in that function call.

the papirus-logo,png in setup.py was so it was moved/findable by the test prog as i didn't know where to put it so it was findable.

the .gitignore for the .pyc's is because it's bad form imho to put "compiled" programs in a repo

shawaj commented 8 years ago

In that function call we do, but in text.py we have got a defined size of 20 don't we?

ukscone commented 8 years ago

yes in text.py you have a default size of 20 (def write(self, text, size = 20):) and no positioning for where text starts (x,y) just start at top and split into screen sized lengths. my version isn't correct but worked for what was needed at the time and to "answer" complaint on twitter. does need redoing to conform to "the papirus way(tm))

ukscone commented 8 years ago

oh and unless i am very wrong to use the xaxis and yaxis in that function you have to use values for all things defined earlier in the call arguments. i don't think you can do papirus.write("hello,world!",,10,10)

you have to fill in the missing value. i think you can do papirus.write("hello,world!",20,10) and have the y axis use the default value

shawaj commented 8 years ago

I'm confused now. What is done the wrong way? On 8 Jun 2016 1:28 a.m., "Russell Davis" notifications@github.com wrote:

yes in text.py you have a default size of 20 (def write(self, text, size = 20):) and no positioning for where text starts (x,y) just start at top and split into screen sized lengths. my version isn't correct but worked for what was needed at the time and to "answer" complaint on twitter. does need redoing to conform to "the papirus way(tm))

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/PiSupply/PaPiRus/pull/38#issuecomment-224453991, or mute the thread https://github.com/notifications/unsubscribe/ADNCuqTB3TUK31zRsT_VB7moOypUIC_hks5qJgyfgaJpZM4IuAsR .

ukscone commented 8 years ago

basically your version of the write() function in text.py had no arguments for user positioning. I put in a quick hack to allow the user to position the text. However it's not neat and is probably not the correct way to do it. I only checked that it did what I expected most of the time, also the arguments are not in the order i'd like them or how you'd normally expect a function to position text to be. Without having two functions write() & write_xy() i can't see how to do it with the way variable number of arguments works in python.

lets take this to email or im

ukscone commented 8 years ago

ah just read some more python docs and I think I see the "correct" way to do it. i'll need to read through the code a bit and try a few things out but I think I have it sussed.

shawaj commented 8 years ago

I can't seem to merge because of conflicts but can just pick out all the changes probably and just load them in, unless you want to fix the conflicts?

ukscone commented 8 years ago

ok i'll do a clean pull of your repo and a clean of my version and try to work out the diffs

shawaj commented 8 years ago

Ok awesome, sounds good on all counts On 15 Jun 2016 5:28 p.m., "Russell Davis" notifications@github.com wrote:

ok i'll do a clean pull of your repo and a clean of my version and try to work out the diffs

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/PiSupply/PaPiRus/pull/38#issuecomment-226242564, or mute the thread https://github.com/notifications/unsubscribe/ADNCul2euvIfCkm5CI3jv5YYe5nY157sks5qMCgWgaJpZM4IuAsR .

francesco-vannini commented 7 years ago

Hi @ukscone, could you please review this PR and see what is still applicable from when you created it and what has already been fixed/changed in the meantime. I'll review after that if this is OK with you. Thanks

francesco-vannini commented 7 years ago

@shawaj a part from the notes everything else has been implemented and documented.

shawaj commented 6 years ago

closing as no longer relevant

shawaj commented 6 years ago

.....and all issues have been fixed that this contains :-)