This bug causes a syntax error when calling the LCDOff/LCD_OFF sub.
Best I can tell the bug was introduced when the LCD_ON and LCD_OFF subs that I added were changed to LCDOn and LCDOFF with defines for for LCD_OFF and LCD_ON.
The bug was introduced because there was already a define for "LCDoff" as a consiant in the code before my modifications. This is on line 88 of the LCD.H file and reads:
LCDOFF is defined as a constant here, but a change was made to also name a sub as LCDOFF in further modification of the LCD.H that I enhanced for speed. I also added the subs LCD_OFF and LCD_ON. These subs were subsequently renamed LCDOFF and LCDON with defines added as follows: #define LCD_ON LCDON and #Define LCD_OFF LCDOFF. This caused a bug because a sub should not have the same name as a previously defined constant.
The current help file in regards to LCDCursor gives us some clues:
From the Help File:
Explanation:
The LCDCursor command will accept the following commands.
ON, OFF, FLASH, FLASHON, LCDOFF
ON will turn the LCD display ON
OFF will turn the LCD display OFF
FLASH will flash the LCD display cursor
FLASHOFF will stop flashing the LCD display cursor
Note that there is no complementary "LCDON, and no mention of what LCDOFF is actually supposed to do. FLASHON is not described.
Here is the actual LCDCursor SUB:
684 sub LCDCursor(In LCDCRSR)
685 ' Revised Evan Venn March 2014
686 'Can be ON, FLASH or OFF, FLASHON
687 Set LCD_RS OFF
688 LCDTemp = 12 '0 or 12
689 If LCDCRSR = 11 Then LCDTemp = 11
690 If LCDCRSR = OFF Then LCDTemp = 12
691 If LCDCRSR = ON Then LCDTemp = 14 '2 or 12
691 If LCDCRSR = FLASH Then LCDTemp = 13 '3 or 12
693 If LCDCRSR = FLASHON Then LCDTemp = 15
694 LCDWriteByte(LCDTemp) '(LCDTemp or 12)
695 ;Wait 5 10us
696
697 end sub
Note that neither LCDOFF nor LCDON are mentioned as options for this sub. FLASHOFF is "described" in the help but is not shoown in the help file single line list, and is not implemented in the LCDCursor Sub. A constant for FLASHOFF is never defined.
A constant is defined for LCDOFF, but it is also not mentioned of implemented in the LCDCursor Sub. There is no mention where what FLASHON is supposed to do as opposed to FLASH.
An older version (02 25 2011) of LCD.H show the LCDCursor sub as follows:
sub LCDCursor(In LCDCRSR)
'Can be ON, FLASH or OFF
Set LCD_RW OFF
LCDTemp = 12 '0 or 12
If LCDCRSR = ON Then LCDTemp = 14 '2 or 12
If LCDCRSR = FLASH Then LCDTemp = 15 '3 or 12
'If LCDCRSR = OFF Then LCDTemp = 0 or 12
LCDWriteByte(LCDTemp) '(LCDTemp or 12)
Wait 5 10us
end sub
Only Flash is defined as a constant in this LCD.H. Note also that there are only three options and that the OFF option is remarked out.
The inconsistencies in the current version of LCD.H need to be corrected so that:
1. There is no compiler syntax error when LCDOFF/LCD_OFF is called.
2. The LCDCursor sub needs to be corrected to support all available options
3. The LCDCursor sub needs to be commented to accurately show valid options
4. Defined constants should be consistent with the LCDCursor Sub and not share names with other subs.
5. Correct information needs to supplied in the Help file that is consistent with what the LCDCursor sub actually does.
What do we want the LCDCursor Sub to actually do?
Do we want it to turn the display on/off as well as control the flashing of the cursor?
Should there be separate sub for turning the display on / off while retaining memory and cursor flash settings?
I can fix this easy enough given answers to the above.
Please Advise
Last edit: William Roth 2015-01-15
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Sorry for delay. Serious issues I have to deal with at home.
As you say this does not look correct. This is causing issues with the compiler. From my review of the ASM we do need to make the changes you recommend.
Hope this answers the questions. This is only my view and you may, of course, adapt.
i. The revised code needs to resolve the issue of LCDOff. I think we have to rename SUB LCDOFF as this may be the easier way to resolve.
ii. As you recommend. LCDCursur should be adapted to support all options, and the revision of the constants to support.
iii. If you make the changes. I can adapt the HELP file. Let me know what to update.
iv. I think we could/should add the LCD on|off sub. A great idea. It would make things simply in GCGB.
Let me know if I have not answered all the questions
Evan
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi Evan,
There is a bug in the Latest LCD.H file.
This bug causes a syntax error when calling the LCDOff/LCD_OFF sub.
Best I can tell the bug was introduced when the LCD_ON and LCD_OFF subs that I added were changed to LCDOn and LCDOFF with defines for for LCD_OFF and LCD_ON.
The bug was introduced because there was already a define for "LCDoff" as a consiant in the code before my modifications. This is on line 88 of the LCD.H file and reads:
LCDOFF is defined as a constant here, but a change was made to also name a sub as LCDOFF in further modification of the LCD.H that I enhanced for speed. I also added the subs LCD_OFF and LCD_ON. These subs were subsequently renamed LCDOFF and LCDON with defines added as follows: #define LCD_ON LCDON and #Define LCD_OFF LCDOFF. This caused a bug because a sub should not have the same name as a previously defined constant.
The current help file in regards to LCDCursor gives us some clues:
From the Help File:
Explanation: The LCDCursor command will accept the following commands. ON, OFF, FLASH, FLASHON, LCDOFF ON will turn the LCD display ON OFF will turn the LCD display OFF FLASH will flash the LCD display cursor FLASHOFF will stop flashing the LCD display cursorNote that there is no complementary "LCDON, and no mention of what LCDOFF is actually supposed to do. FLASHON is not described.
Here is the actual LCDCursor SUB:
Note that neither LCDOFF nor LCDON are mentioned as options for this sub. FLASHOFF is "described" in the help but is not shoown in the help file single line list, and is not implemented in the LCDCursor Sub. A constant for FLASHOFF is never defined.
A constant is defined for LCDOFF, but it is also not mentioned of implemented in the LCDCursor Sub. There is no mention where what FLASHON is supposed to do as opposed to FLASH.
An older version (02 25 2011) of LCD.H show the LCDCursor sub as follows:
sub LCDCursor(In LCDCRSR) 'Can be ON, FLASH or OFF Set LCD_RW OFF LCDTemp = 12 '0 or 12 If LCDCRSR = ON Then LCDTemp = 14 '2 or 12 If LCDCRSR = FLASH Then LCDTemp = 15 '3 or 12 'If LCDCRSR = OFF Then LCDTemp = 0 or 12 LCDWriteByte(LCDTemp) '(LCDTemp or 12) Wait 5 10us end subOnly Flash is defined as a constant in this LCD.H. Note also that there are only three options and that the OFF option is remarked out.
The inconsistencies in the current version of LCD.H need to be corrected so that:
1. There is no compiler syntax error when LCDOFF/LCD_OFF is called.
2. The LCDCursor sub needs to be corrected to support all available options
3. The LCDCursor sub needs to be commented to accurately show valid options
4. Defined constants should be consistent with the LCDCursor Sub and not share names with other subs.
5. Correct information needs to supplied in the Help file that is consistent with what the LCDCursor sub actually does.
What do we want the LCDCursor Sub to actually do?
Do we want it to turn the display on/off as well as control the flashing of the cursor?
Should there be separate sub for turning the display on / off while retaining memory and cursor flash settings?
I can fix this easy enough given answers to the above.
Please Advise
Last edit: William Roth 2015-01-15
Is anyone here? Is everyone on vacation?
Sorry for delay. Serious issues I have to deal with at home.
As you say this does not look correct. This is causing issues with the compiler. From my review of the ASM we do need to make the changes you recommend.
Hope this answers the questions. This is only my view and you may, of course, adapt.
i. The revised code needs to resolve the issue of LCDOff. I think we have to rename SUB LCDOFF as this may be the easier way to resolve.
ii. As you recommend. LCDCursur should be adapted to support all options, and the revision of the constants to support.
iii. If you make the changes. I can adapt the HELP file. Let me know what to update.
iv. I think we could/should add the LCD on|off sub. A great idea. It would make things simply in GCGB.
Let me know if I have not answered all the questions
Evan
I will make the changes, test, and send to you for review. Should I send via email ?
Last edit: William Roth 2015-01-17
A good idea. We can then post the final release file to the repository.
Evan,
New file was sent via email at 9:37AM Today
This bug has been resolved. See the "Sticky" dated 28/02/2015 for the latest version
Last edit: William Roth 2015-01-29