kbrepeatdelay and kbrepeatrate for cbm targets #452

mrdudz · 2017-06-16T22:34:38Z

quick PR to get things going... (not really ready yet)

since it came up on the list recently - do we want these functions? :) are the names OK or not? should prototypes go into conio.h ?

polluks · 2017-06-20T17:47:56Z

asminc/plus4.inc

@@ -33,6 +33,11 @@ FKEY_COUNT      := $55D         ; Characters for function key
 FKEY_SPACE      := $55F         ; Function key definitions
 FKEY_ORIG       := $F3D2        ; Original definitions

+;FIXME: he?! these ok? :o)


No, they are $540, $541 and $542.

polluks · 2017-06-20T18:05:14Z

asminc/pet.inc

@@ -31,6 +31,11 @@ BASIC_BUF_LEN   = 81            ; Maximum length of command-line

 KEY_BUF         := $26F         ; Keyboard buffer

+;FIXME: these are wrong?


Indeed.
40 columns: $3ee, $3ea, $3e9
80 columns: $e4, $e5, $e6

mrdudz · 2017-06-21T22:06:08Z

more?

how should we handle the difference between 40- and 80- column models? they are different targets anyway, right? so the different constants could be achived using some ifdef magic? or another .inc? or what?

mrdudz · 2017-06-22T14:26:22Z

i was thinking of the PET targets :)

polluks · 2017-06-22T15:15:20Z

Oh I see. Check in target pet for SCR_LINELEN.

polluks · 2017-06-22T16:01:43Z

Just for Uz...
cbm510: $D7 and $D8
http://www.von-bassewitz.de/uz/oldcomputers/p500/rom500.s.html

greg-king5 · 2017-06-22T19:04:15Z

How should we handle the difference between 40- and 80- column PET/CBM models?

(cc65 has a single -t pet target.) cputc() tests it during run-time:
https://github.com/cc65/cc65/blob/master/libsrc/pet/cputc.s#L84-L86

So, you will need to define constants like KBDREPEAT40 and KBDREPEAT80, and to choose the right one, at run-time.

I think that it makes sense to put kbdrepeat()'s prototype in <cbm.h>. The others are tick count-down timers; the Kernal changes them!

mrdudz · 2017-06-23T16:30:04Z

I think that it makes sense to put kbdrepeat()'s prototype in <cbm.h>.

mmmh - but that would indicate they are CBM specific (which they hopefully wont be on the long run)

polluks · 2017-06-23T16:44:12Z

Well, the functions are something like get_tv(), for example the NES will never get a kbdrepeat().

mrdudz · 2017-06-23T16:47:54Z

by that logic, also kbhit and cgetc (and probably more) shouldnt be in conio either :)

polluks · 2017-06-23T17:37:20Z

I see, there's something inconsistent.
Take a look at hello.c our conio reference
#if defined(__NES__) || defined(__PCE__) || defined(__GAMATE__)
conio expects an input, therefore its name, but you don't have a keyboard this time.

Because many targets have an implementation, get_tv() should move to conio?
The question is whether kbdrepeat() will be useful without conio or not IMHO.

oliverschmidt · 2017-06-24T17:33:40Z

Hi,

Maybe I don't have a (good enough) overview but...

I think this isn't a precise science. It's rather about a sense of what is appropriate.

I'd say something that is available on (almost) all targets (think keyboard) and/or can be emulated on (almost) all targets (think F-keys) belongs into a shared header structured by topics (conio, tgi, mouse, ...). Something that is available on one (or few) targets belongs into the target header(s).

Let's not discuss things here which have already found their places - even if those places might not always be optimal and/or follow the guide I laid out above.

Those keyboard repeat setting functions seem pretty target specific to me. I'd guess no non-CBM target supplies something similar. So I personally clearly see them in cbm.h.

Given that they are not considered part of conio I personally think that names starting with 'kb' are fine. I would however rather use the prefix 'kbd' instead of just 'kb'. Or are there things in the CBM world (i.e. BASIC commands or alike) that make the names suggested a premier choice?

Regards,
Oliver

oliverschmidt · 2017-06-24T17:33:59Z

Ooops, didn't want to close!

greg-king5 · 2017-06-24T21:15:36Z

Also, ... portable programs that use stdio might want to use kbdrepeat() to make CBM keyboards act the way that other keyboards act.

We should have also some defined constants for kbdrepeat():

#define KBDREPEAT_CURSOR 0x00
#define KBDREPEAT_NONE 0x40
#define KBDREPEAT_ALL 0x80

oliverschmidt · 2017-06-25T10:48:55Z

Also, ... portable programs that use stdio might want to use kbdrepeat() to make CBM keyboards act the way that other keyboards act.

I think I understand that this statement refers to your proposal to "set" "something" on the keyboard by default on conio initialization. There were however strong oppsite opinions.

Is it possible to elaborate on the topic in a way a non-CBM insider understand the issue? I.e. I presume that a "normal" computer in a "normal" scenario (like i.e. working in the BASIC screen editor) behaves like this: I press a key and the key is accepted once - but if I continue to hold down the key then after some delay (!) the key starts to auto repeat until I release the key.

mrdudz · 2017-06-25T14:41:30Z

on CBM targets the regular alphanumeric keys do not have any repeat at all - only the "editor" keys (cursor, space, insert/delete). why space belongs to them, i have no idea :)

oliverschmidt · 2017-06-25T14:46:28Z

Thanks - and do I understand it correctly that it is possible to have repeat for the "regular" keys as well? Does the repeat have the "usual" delay before starting? If both question are answered with 'yes' then why isn't it desirable to activate repeat for all keys by default (just like we i.e. switch to lower case output by default) ?

mrdudz · 2017-06-25T14:48:33Z

yes its possible. and yes they will have the initial delay. however i dont think it should be default, since its not what a typical c64 users expects - most programs do not work this way.

oliverschmidt · 2017-06-25T14:50:17Z

Thanks for the explanation. So it's about soft facts and meeting expectations...

greg-king5 · 2017-06-26T08:01:46Z

I'm not aware of any programs that demand that the alpha-numerics never repeat. I find it difficult to imagine that a program would fail if a key was to repeat -- after we held it down for awhile. If a program like that does exist, then it's very unusual; so, it should tell people, "don't hold down any keys". And, it should disable repeating on keyboards that support that switch.

I still think that -- portable -- programs should be able to have some consistent expectations about the environments on which they run. So, ... programs should expect to be able to print lower-case text, with capital letters where they're needed. And maybe, they should expect that everybody's keyboards repeat every key.

By the way, Commodore wasn't consistent with their models! The C128 and the Plus4, for example, do repeat every key, by default.

oliverschmidt · 2017-06-26T08:56:58Z

I personally think that Greg's argument is valid. However to be honest it's just not important enough to me to enter a dispute about that topic.

Maybe a reasonable compromise would be to have a cc65 constructor that activates the repeat feature and that is as easily as possible to be activated by the user who wants it. Ideally from the build command line...

oliverschmidt

Beside the "FIXME" question I'd really like to see some - at least rudimentary - documentation.
The question about enabling the keyboard repeat by default seems to have lost momentum so I don't see it blocking this pull request (anymore).

oliverschmidt · 2017-08-06T10:36:16Z

asminc/pet.inc

@@ -31,6 +31,18 @@ BASIC_BUF_LEN   = 81            ; Maximum length of command-line

 KEY_BUF         := $26F         ; Keyboard buffer

+;FIXME: we must somehow handle the difference between the two - how?


Is this pull request supposed to be merged with this FIXME still in place?

i don't know.... would that be acceptable? i don't like the idea a lot myself.... but i don't know to fix this either. are there preprocessor symbols for 40cols vs 80cols PETs? or even different libraries? can it be checked at runtime, and how? i have no idea about either :)

mrdudz · 2017-08-06T18:35:05Z

added some docs. i cant test if its ok because linuxdocs fail to build the docs here, for whatever reason

oliverschmidt · 2017-08-07T15:00:33Z

added some docs.

:-)

i cant test if its ok because linuxdocs fail to build the docs here, for whatever reason

At least there are no errors/warnings according to Travis CI...

mrdudz · 2017-08-07T15:04:58Z

no, but here there are - and when i fix them it will fail on CI :) different linuxdoc versions, i guess

greg-king5 · 2017-08-08T01:28:29Z

Those functions are fastcall. Their prototypes must be marked with __fastcall__, so that they will be called properly even if --all-cdecl is used. And, their funcref descriptions must have the usual note that says that a fastcall function must be used only in the presence of a prototype.

The repeat-delay and the repeat-rate variables aren't "settings". Instead, they are active counters. When a key is pressed, the Kernal changes them 60 times every second. Therefore, there is no "original value" that should be restored by a program. So, their functions should return void.

mrdudz · 2017-08-19T17:11:46Z

The repeat-delay and the repeat-rate variables aren't "settings". Instead, they are active counters. When a key is pressed, the Kernal changes them 60 times every second. Therefore, there is no "original value" that should be restored by a program. So, their functions should return void.

ewww. i could have sworn those are persistant settings. like this the two functions are actually not quite useful at all (you'd have to call them after each keyboard scanning) - i'd rather just remove them. no idea why i have even added them back then :=)

mrdudz · 2017-08-21T01:06:40Z

mmmh shrug

oliverschmidt · 2017-08-21T06:51:58Z

Sorry - what is wrong? Should I not have this merged?

mrdudz · 2017-08-21T16:15:51Z

as gregg pointed out, the functions that are supposed to set the repeat delay and repeat rate do not work as advertised (and i see no simple way to fix that unfortunately) - so they should probably get removed before someone uses them and wonders that they dont do what they are supposed to do :=)

oliverschmidt · 2017-08-22T06:59:13Z

Ah, now I see. Your last two commits are listed below your comment starting with 'ewww' so I presumed your commits were meant to "adjust" what was "wrong". I admit that I was too busy trying to make everybody happy regarding the joystick interface to really try to understand what Greg and you were up to.

So do I get it right this time that this pull request should just have been closed instead of being merged? So a full revert is the right thing to do now?

mrdudz · 2017-08-22T15:40:01Z

closed? no.... "someone" should delete the two useless functions, then it can be merged :)

greg-king5 · 2017-08-23T04:23:10Z

Maybe they aren't totally useless. I haven't tested it; but, maybe they can be used to zero those counters immediately after calling cgetc(). It might make the keyboard look more like a joystick; that is, we might get immediate and very rapid key repeating.

mrdudz · 2017-08-23T16:17:32Z

that doesnt really work as intended... and resetting the live counters the hard way like that inside your keyboard polling loop.... urks. that sounds like ugly hackery to me that i dont want to encourage :)

As discussed in #452 after my premature merge the two functions in question don't work as expected. Additionally I adjusted several style deviations in the pull request in question.

oliverschmidt · 2017-08-30T14:41:00Z

"someone" should delete the two useless functions

Done, see 4aa1949

mrdudz · 2017-08-30T22:46:17Z

Am Mittwoch, 30. August 2017, 16:41:04 CEST schrieb Oliver Schmidt:

> "someone" should delete the two useless functions Done, see 4aa1949 e

great :) so ... what happened to greggs cpeekxy()? :=)

…

-- http://www.hitmen-console.org http://magicdisk.untergrund.net http://www.pokefinder.org http://ar.pokefinder.org In C++, friends can access each others' private members.

mrdudz added 2 commits February 28, 2016 22:13

initial import from old sources

85760e0

fixed some stuff, compiles again :)

a42feca

polluks reviewed Jun 20, 2017

View reviewed changes

fixed zp locations provided by polluks :)

e985e0b

oliverschmidt closed this Jun 24, 2017

oliverschmidt reopened this Jun 24, 2017

oliverschmidt requested changes Aug 6, 2017

View reviewed changes

mrdudz added 3 commits August 6, 2017 18:09

added prototypes and proposed constants

bd92088

added documentation for the kbrepeat... functions

dafe3c1

resolve conflict

99e6dd1

Merge branch 'master' into kbrepeat

5dcebbc

mrdudz added 2 commits August 19, 2017 19:14

use fastcall

9609570

add note on fastcall

3c35588

oliverschmidt approved these changes Aug 20, 2017

View reviewed changes

oliverschmidt merged commit b5a4e56 into cc65:master Aug 20, 2017

greg-king5 mentioned this pull request Sep 5, 2017

Make the CBM Pet kbrepeat() work on both 40-/80-column machines. #482

Merged

mrdudz mentioned this pull request Dec 7, 2017

New OSI input routine that fixes issue #511. #543

Closed

mrdudz deleted the kbrepeat branch December 27, 2017 17:34

		@@ -31,6 +31,11 @@ BASIC_BUF_LEN = 81 ; Maximum length of command-line

		KEY_BUF := $26F ; Keyboard buffer

		;FIXME: these are wrong?

		@@ -31,6 +31,18 @@ BASIC_BUF_LEN = 81 ; Maximum length of command-line

		KEY_BUF := $26F ; Keyboard buffer

		;FIXME: we must somehow handle the difference between the two - how?

kbrepeatdelay and kbrepeatrate for cbm targets #452

kbrepeatdelay and kbrepeatrate for cbm targets #452

Conversation

mrdudz commented Jun 16, 2017 • edited

polluks Jun 20, 2017

Choose a reason for hiding this comment

polluks Jun 20, 2017

Choose a reason for hiding this comment

mrdudz commented Jun 21, 2017

mrdudz commented Jun 22, 2017

polluks commented Jun 22, 2017

polluks commented Jun 22, 2017

greg-king5 commented Jun 22, 2017

mrdudz commented Jun 23, 2017

polluks commented Jun 23, 2017

mrdudz commented Jun 23, 2017

polluks commented Jun 23, 2017

oliverschmidt commented Jun 24, 2017 • edited

oliverschmidt commented Jun 24, 2017

greg-king5 commented Jun 24, 2017

oliverschmidt commented Jun 25, 2017 • edited

mrdudz commented Jun 25, 2017

oliverschmidt commented Jun 25, 2017

mrdudz commented Jun 25, 2017

oliverschmidt commented Jun 25, 2017

greg-king5 commented Jun 26, 2017 • edited

oliverschmidt commented Jun 26, 2017

oliverschmidt left a comment

Choose a reason for hiding this comment

oliverschmidt Aug 6, 2017

Choose a reason for hiding this comment

mrdudz Aug 6, 2017

Choose a reason for hiding this comment

mrdudz commented Aug 6, 2017

oliverschmidt commented Aug 7, 2017

mrdudz commented Aug 7, 2017

greg-king5 commented Aug 8, 2017

mrdudz commented Aug 19, 2017

mrdudz commented Aug 21, 2017

oliverschmidt commented Aug 21, 2017

mrdudz commented Aug 21, 2017

oliverschmidt commented Aug 22, 2017 • edited

mrdudz commented Aug 22, 2017

greg-king5 commented Aug 23, 2017

mrdudz commented Aug 23, 2017

oliverschmidt commented Aug 30, 2017

mrdudz commented Aug 30, 2017 via email

mrdudz commented Jun 16, 2017 •

edited

oliverschmidt commented Jun 24, 2017 •

edited

oliverschmidt commented Jun 25, 2017 •

edited

greg-king5 commented Jun 26, 2017 •

edited

oliverschmidt commented Aug 22, 2017 •

edited