hercules-390 / hyperion

Hercules 390
Other
251 stars 70 forks source link

Printer FCB option parsing is wrong #40

Open ghost opened 10 years ago

ghost commented 10 years ago

Old (original) style was: "fcb=66010713192531374361495561":

which yields:    66 1 7 13 19 25 31 37 43 61 49 55 61
int FCBMASK[] = {66,1,7,13,19,25,31,37,43,63,49,55,61};

Notice the old style is a string of 26 digits (13 pairs), which are NOT separated by commas. The current parsing code presumes comma separators which is WRONG and incorrectly causes the old format to always be rejected.

Also notice channel 9 and 12 are on same line number 61. This is valid for old 1403 paper carraige-control tapes, but is invalid on newer 3211 FCB image printers which support only one channel defined per line.

We need to provide support for BOTH the old (original) format and the newer format FCB= option, as well as providing new support for virtual "carraige-control tapes" wherein not only may any line have any number of channels defined on it, but wherein any channel may also be defined on any number of lines.

ghost commented 10 years ago

On 26 Aug 2014, at 18:03, BGMNT notifications@github.com wrote:

Notice the old style is a string of 26 digits (13 pairs), which are NOT separated by commas. The current parsing code presumes comma separators which is WRONG and incorrectly causes the old format to always be rejected.

IT IS NOT WRONG … it is just different , it seemed easier to understand the layout

We need to provide support for BOTH the old (original) format and the newer format FCB= option

I ALREADY TOLD THAT I WAS GOING TO FIX IT

, as well as providing new support for virtual "carraige-control tapes" wherein not only may any line have any number of channels defined on it, but wherein any channel may also be defined on any number of lines.

seems overshooting to me , I looked into it and instead of a simple array channel/line number it would be necessary to have an array 12 by 256 and search into it

Fish-Git commented 10 years ago

Well, I personally consider any change that causes valid configuration file statements to be rejected to be a bug (i.e. "wrong").

The option was originally documented as having a specific format and the original code was written to parse and understand that format and people started using that format in their configuration files. Since originally valid configuration file statements (which were never documented as having been deprecated) are now being rejected, the code in question is, in my opinion, not "just different" but is in fact wrong.

I do sincerely apologize however if such a choice of words upset you. I did not mean to offend you, Enrico. You should know that. I like you. You are a good friend. I was simply trying to document an existing bug that needs to be fixed! That's all.

I too have also been working on this issue (since it didn't seem that hard to do) and still have a working implementation ready to commit (or had; it's been a while since I've worked on it) that addresses all three issues: the old/new FCB option format as well as the new CCTAPE= option. I have not worked on it for some time, but I still have the code which I believe is complete. I need to review it again just to be sure.

I was going to just go ahead and commit it since it has been taking you this issue has been open for so long, but will hold off on doing so if you already have your own fix ready to commit. I would be happy to share my code with you if it would help you.

Fish-Git commented 10 years ago

I just realized something:

Is my occasional use of capitalized words for emphasis (instead of surrounding them with, say, underscores or asterisks or slashes) causing you to perhaps think I'm "yelling" (shouting) at you? Is that what's going on?

If so, I'm very sorry! It's a very old and well established writing style of mine that I picked up years ago and one that I honestly never considered before might inadvertently be misinterpreted by others.

I'll have to work on that.

(Sheesh! Sorry about that folks!)

ghost commented 10 years ago

On 26 Aug 2014, at 18:51, Fish-Git notifications@github.com wrote:

Assigned #40 to @herc4mac.

— Reply to this email directly or view it on GitHub.

please hold a bit before doing anything, everything is going to be part of a BIIG rework of printer.c and cardpch.c ( same level of functionality and -probably more important- code sharing )

cheers enrico

Fish-Git commented 10 years ago

please hold a bit before doing anything, everything is going to be part of a BIIG rework of printer.c and cardpch.c ( same level of functionality and -probably more important- code sharing )

10-4.

"Fish" (David B. Trout) Software Development Laboratories http://www.softdevlabs.com (TEMP): david.b.trout@gmail.com