Code verification errors 16F1847

[wjsmarine]

Hi All,

I have written a program developed on an 18F2620 and then ported across to a 16F1847 using 7786 words of program space. It uses DTInt's, a rotary encoder (menu selection) and a Nokia 3310 display with its include "LCD_3310v32 WJS.inc" (has some edited characters).

The 18F has no problems programming or verifying and runs correctly but with the 16F it programs okay without any errors (only page boundaries warnings) however fails verification always at the same locations e.g. "code verify error at 11BC. 1761 should be 5761" and "11BD 3465 should be 7465", "11BE 3244 should be 7244", "11BF 2179 should be 6179", etc,. I notice there is always a 4xxx difference between the numbers but don't know the significance. I've tried different hardware programmers (both faultless on every other task) with same results and multiple (new) 16F1847 to no avail.

The only differences made to the program are to change DTInt's Include files from the 18 version to the 14 version and add the wsave line. I have already manufactured and populated the pcb to take a DIL18 else would just use the skinnydip 28 and be done with it - making an adapter is clumsy and brings height issues with it so I'd like to find and fix the problem.

I don't have any other 8k DIL18 16F PICs to try in place of the 16F1847. I'm using PBPX 3.0.6.4 and MCSP 5.0.0.5 with MPASM (where do I find what version of MPASM the compiler is using?).

Can somebody offer any advice on the verification problem or suggest the next thing to try?

Kind regards,
Bill

[richard]

I'm using PBPX 3.0.6.4

why ?

3.010.4 is the latest freely available version of pbp3.0




The only differences made to the program are to change DTInt's Include files from the 18 version to the 14 version and add the wsave line
that chip has auto context save Wsave is not necessary and could cause errors




(where do I find what version of MPASM the compiler is using?).

see below

[wjsmarine]

Hi Richard,

Thanks for your help.

I remarked the wsave but it made no difference, I still have the exact same verification issue. I see I'm using MPASM 5.45 when compiling - is this the cause?

I did a search for 'wsave' and 'auto context saving' in the 16F1847 data sheet but nothing found - please enlighten me so I can check before using again on different chips.

As to the version of PBP3 I'm using... is it freely available or is it a purchased upgrade? I'm trying to hold off as long as I can (money's tight, I'm out of work) before paying out again unless I really need to.

Kind regards,
Bill
(in Perth)

[richard]

I did a search for 'wsave' and 'auto context saving' in the 16F1847 data sheet but nothing found

hi bill
basically any chip that has auto context saving requires no additional code/vars to save the context, for the priority interrupt anyway.
wsave psave fsave... are a legacy of old chips .


As to the version of PBP3 I'm using... is it freely available or is it a purchased upgrade?

its free upgrade for licenced pbp3 users, you are quite a few versions behind ditto for mpasm . it may be the issue.
but it won't hurt to upgrade

[richard]

data sheet page 82



8.5 Automatic Context Saving
Upon entering an interrupt, the return PC address is
saved on the stack. Additionally, the following registers
are automatically saved in the Shadow registers:
• W register
• STATUS register (except for TO and PD)
• BSR register
• FSR registers
• PCLATH register
Upon exiting the Interrupt Service Routine, these
registers are automatically restored. Any modifications
to these registers during the ISR will be lost. If modifications
to any of these registers are desired, the
corresponding shadow register should be modified and
the value will be restored when exiting the ISR. The
shadow registers are available in Bank 31 and are
readable and writable. Depending on the user’s
application, other registers may also need to be saved.

[Art]

Are you sure you’ve changed the destination device in the project from the 18F to the 16F part?
The only reason I can think of to blame software being written to the device for the device not programming, is writing invalid data to the device.

16F devices have a 14 bit instruction set, and 18F have 16 bit instruction set. That’s not a hard rule, but generally true.
Given that it’s chopping one of the upper two bits off, that’s suspicious.
The first example you gave looks like a specific subtract instruction op code that is not supported with 16F.
They both have subtract instructions, but the 16F can’t subtract from RAM, and load the destination in a single instruction.

I’m a little baffled though because I wouldn’t expect an 18F hex file to be able to load in the programmer software when a 16F part is selected or detected.
Just in case, why not read the program hex from he 18F part and try to load that straight to the 16F part to check the result?

[wjsmarine]

Thanks Richard and Art,

I found the Automatic Context Saving info in the data sheet after refining the search criteria, that's a lesson learned first up.

The MeLabs website mentions paid upgrades but I don't see anything for free even if a licenced owner (I am) - I've emailed Charles Leo for clarification but he's overworked so not expecting a reply soon.

Art, there's some confusion with your last paragraph - I'm not loading an 18F hex file into a 16F part, both are different programs with different resulting hex files except the 18F programs the chip without any problems and runs on a breadboard faultlessly. The 16F programs but suffers those verification checks. As mentioned, the only changes are those concerning the DTInt's files differences between the two families.

I'm looking into upgrading MPASM to see if it changes things - what should I be looking for? The only reference I have to go by is I see on my laptop \PBP3\MPLAB\MPLAB Tools v8.85.msi that I can't remember installing... I'm assuming the MPASM v5.45 I see at the top of the compile box came from the msi install - or am I wrong? Is it MPLAB X I need to download and install or am I completely missing the plot here?

On another tack, if I buy an upgrade does it include the appropriate version of MPASM? If not, how does one ever know when or what version of MPASM is needed? Confusion reigns.

Thanks and regards,
Bill

[richard]

its all there on the melabs web site
http://pbp3.com/download_legacy.html

[Dave]

I have used a few of these 16F1847's to replace a few old 16F628's on a couple of REAL OLD projects... They work a treat...

[wjsmarine]

Hi Richard,

Charles also responded to my email and provided the same information, thanks to both of you although I'm still wondering why I couldn't find that info myself.

I downloaded and installed MPLABX, checked the appropriate box in the program options and recompiled the code for the 16F1847 (MPASM v5.77 now indicated). I am now greeted with error messages (not there when MPASMX unchecked):
"Message[303] C:\PBP\BILL'S RETIC.ASM 1191 : Program word too large. Truncated to core size. (5761)"
followed by another 77 lines of the same but with end number changes.

Does this mean the chip program space is not big enough? It compiles to 7789 words used while the 18F2620 compiles to 14146 bytes.

While I'm here... how does one capture/copy the results messages? I appear to highlight it (turns black) but ^c has no effect.

Thanks for your patience and sharing of knowledge.

Kind regards,
Bill

[richard]

"Message[303] C:\PBP\BILL'S RETIC.ASM 1191 : Program word too large. Truncated to core size. (5761)"
followed by another 77 lines of the same but with end number changes.

Does this mean the chip program space is not big enough? It compiles to 7789 words used while the 18F2620 compiles to 14146 bytes.


it sounds more like a calculated jump or memory offset has gone out of limits, maybe if you post the entire error message
but sometimes the clues are pretty vague





While I'm here... how does one capture/copy the results messages? I appear to highlight it (turns black) but ^c has no effect

its not quite a clean copy though

[wjsmarine]

Hi Richard,

Thanks for taking the time to help.

I was too quick in my last post - I had not upgraded to PBP3.0.10 when I did those tests, I guessed it was a MPASM issue...

I then installed PBP3.0.10 and repeated the exercise - it's all good! No error messages and zero verification errors when burning the chip. In hindsight I should have done everything first and then posted - Doh!

Thanks for the info on copying the results area, a great resource I'll make a note of. I hope these findings are useful for others who may experience similar difficulties.

Thanks again and best regards,
Bill

[wjsmarine]

Hi All,

Since my last post on this subject I've tested the 16F1847 in my custom pcb following a good compile and verification (after upgrading MPLAB/MPASM/PBP3).

The results are not good (please see attached files). The hardware is confirmed good as the GLCD displays "Hello World" correctly but fails miserably when the @ printstr x,y,"xxxxx" command is invoked. I've had good success using this and more on a 16F88 pic and I need something with more codespace.

If there are any takers out there with advice on how to crack this, you have my attention...





'************************************************* ****************************
'* Name : Bill's Retic testing.pbp #### NON Working Code #### *
'* Author : WJ Sherwood *
'* Date : 02/05/18 *
'* Device : 16F1847 (8k code space) *
'* Version : 1.0 2588 words with PBP 3.0.10.4 *
'* Notes : *
'* : Hello World works fine but @ PrintStr 6,3,"Reticulation" does *
'* : not and causes strange effects. *
'* : *
'* : *
'************************************************* ****************************
'
' Connections as follows:
'
' PIC(F1847) OTHER
' PortA.2 (pin 1) SDA on Nokia module.
' PortA.3 (pin 2) D/C on Nokia module.
' PortA.4 (pin 3) RST on Nokia module.
' MCLR PortA.5 (pin 4) 10k pullup to 5v via diode for ICSP.
' Vss Ground (pin 5) CS on Nokia module.
' PortB.0 (pin 6) Spare.
' PortB.1 (pin 7)
' PortB.2 (pin 8)
' (CCP1) PortB.3 (pin 9) Spare.
' PortB.4 (pin 10)
' PortB.5 (pin 11)
' PortB.6 (pin 12)
' PortB.7 (pin 13) Spare.
' Xtal PortA.6 (pin 15) Spare. LCD pin hard grounded.
' Vdd 5 volts (pin 14)
' Xtal PortA.7 (pin 16) LED indicator and optocoupler driver for Triac/solenoid valve.
' PortA.0 (pin 17) LED backlight on Nokia module.
' PortA.1 (pin 18) SCK on Nokia module.

' NOKIA 3310 LCD (eBay) MODULE PINOUT
' as viewed from the FRONT (left -> right)
' ---------------------
' | oooooooo | Note actual display area is 30x21mm.
' | 1 8 |
' ---------------------
' | |
' | |
' | |
' | |
' ---------------------
' | oooooooo |
' | 1 8 |
' ---------------------
'
'1 !rst portA.4
'2 !cs Gnd #### works fine grounded, saved a pin here.
'3 d/!c portA.3
'4 sda portA.2 (pulled high 4k7). Runs okay without it (pullup on pcb?).
'5 sclk portA.1
'6 vcc 3.3v
'7 Led portA.0 Gnd to light - connects to vcc (via smd resistor if solder JP near pin 8 top), max 3v3 supply!
'8 gnd

'================================================= ================================================== ==
' CONFIGURE DEVICE
'================================================= ================================================== ==

@ ERRORLEVEL -306 ; turn off crossing page boundary message

#CONFIG ; F1847.
__config _CONFIG1, _FOSC_INTOSC & _WDTE_ON & _PWRTE_ON & _MCLRE_ON & _CP_OFF & _BOREN_OFF
__config _CONFIG2, _PLLEN_OFF & _LVP_OFF
#ENDCONFIG

' -----[ Initialization ]------------------------------------------------------------------------------------------------

clear ' ****** Clear statement must be before any INC file.******

Include "MODEDEFS.BAS" ' Include Shiftin/out modes.
include "LCD_3310v32 WJS.inc" ' Edited for current hardware connections.

TRISA = %00000000 ' All outputs.
TRISB = %00000000 ' All outputs.
ANSELA = 0 ' Turn off analog port A
ANSELB = 0 ' Turn off analog port B
CM1CON0 = 0 ' Comparators off.
CM1CON1 = 0
CM2CON0 = 0 ' Comparators off.
CM2CON1 = 0
CPSCON0 = 0 ' Capacitive sense module off.
OSCCON = %01101010 ' 4Mhz (F1847).
' INTCON = %00001000 ' B.3 = 1 - Interrupt on change Enable bit.
' IOCBN = %00110000 ' Enable IOC falling edge on PORTB.4-5 (pulled up).
' IOCBP = 0 ' Disable IOC rising edge on PORTB.

' Stored onboard E2...
Message Data "Bill's DIY Retic testing"

'================================================= ================================================== ==
' PIN ASSIGNMENTS, SYSTEM CONSTANTS, TEMPORARY VARIABLES
'================================================= ================================================== ==
' Alias pins
Triac var PortA.7 ' Optocoupler drives the triac/solenoid.

GOTO Satu ' Jump over the subroutines area.

'================================================= ================================================== ==
' Subroutines here in the first page...
'================================================= ================================================== ==
Splash: ' Show opening screen.
'@ PrintStr 12,2,"Bill's DIY" ; the single quote causes problems within the asm - workaround below.
'@ PrintStr 12,2,"Bill" ; First part.
' Lcd_Data = "'" ' Note byte size single char.
' Gosub Lcd_SendChar '
'@ PrintStr 42,2,"s DIY" ; Final part.
@ PrintStr 6,3,"Reticulation" ; Line 4.
@ PrintStr 18,4,"02May18" ; Line 5.
@ PrintStr 0,5,"Push to select" ; Line 6.
return

'================================================= ================================================== ==
' Program area...
'================================================= ================================================== ==
Satu: ' Operation when system first powered up.
pause 1000
pulsout Triac, 20000 ' Heartbeat for testing, 200ms.
gosub Lcd_Init ' Initialize LCD.
gosub Scr_Normal ' Normal Black on White.
Gosub Splash ' Show opening screen.
pause 3000 ' 3 seconds.

gosub Lcd_Clear ' Clear contents of screen
pause 50

PosX = 30:PosY = 0:gosub Lcd_GotoXY ' position 30 of 84 in the x direction, row 0 (of 0 to 5)
Lcd_Data = "H"
gosub Lcd_SendChar
Lcd_Data = "E"
gosub Lcd_SendChar
Lcd_Data = "L"
gosub Lcd_SendChar
Lcd_Data = "L"
gosub Lcd_SendChar
Lcd_Data = "O"
gosub Lcd_SendChar

PosX = 30:PosY = 1:gosub Lcd_GotoXY ' position 30 of 84 in the x direction, row 1 (of 0 to 5)
Lcd_Data = "W"
gosub Lcd_SendChar
Lcd_Data = "O"
gosub Lcd_SendChar
Lcd_Data = "R"
gosub Lcd_SendChar
Lcd_Data = "L"
gosub Lcd_SendChar
Lcd_Data = "D"
gosub Lcd_SendChar
Lcd_Data = "!"
gosub Lcd_SendChar

pause 1000 '
gosub Lcd_Clear ' Clear contents of screen
goto Satu ' Repeat

end




8631

FWIW there is a similar thread I posted under "Using Nokia LCD" today concerning the troublesome 16F1847 but using mister_e's Nokia wrapper (which succeeds Scale Robotics' earlier macro used in this Hello World example)... I'm sure there is a common problem with both.

Best regards,
Bill

[richard]

the problem is in this macro

ASM
PrintStr macro x, y, Str
local TheString, OverStr
goto OverStr
TheString
data Str, 0
OverStr
MOVE?CB x, _PosX
MOVE?CB y, _PosY
MOVE?CW TheString, _Addr
L?CALL _StringOut
endm
ENDASM

its not that simple going from code for pic18 to pic16
the word size for core 14 pics ie pic16 is 14bits
the "data" directive attempts to write two 8 bit pieces of data into a 14bit slot ,hence the end falls off



you could change it to use the "da" directive to pack two 7 bit pieces of data into that space [its ok for 7bit ascii]
but then you need to unpack the data when you read it back

[wjsmarine]

Hi Richard,

Hmmm, then why does it work perfectly on an F88?

Can you give me an example how I can change to "da" and then unpack please?

Kind regards,
Bill

[richard]

if you look at the lst file the compiler detects a problem for the 16f88 and still generated the wrong result

16f88 lst




0934 M TheString
Message[303]: Program word too large. Truncated to core size. (5265)
Message[303]: Program word too large. Truncated to core size. (7469)
Message[303]: Program word too large. Truncated to core size. (6375)
Message[303]: Program word too large. Truncated to core size. (6C61)
Message[303]: Program word too large. Truncated to core size. (7469)
Message[303]: Program word too large. Truncated to core size. (6F6E)
0934 1265 3469 2375 M data "Reticulation", 0
2C61 3469 2F6E
0000


16f1847


0000 M local TheString, OverStr
0971 2979 M goto OverStr
0972 M TheString
0972 1265 3469 2375 M data "Reticulation", 0
2C61 3469 2F6E
0000
0979 M OverStr


pic18 the correct result


02715 PrintStr 6,3,"Reticulation" ; Line 4.
0000 M local TheString, OverStr
0011FE EF08 F009 M goto OverStr
001202 M TheString
001202 6552 6974 7563 M data "Reticulation", 0
616C 6974 6E6F
0000
001210 M OverStr




maybe it was ok in older versions of complier , I doubt it though ,wrong is wrong

[richard]

Can you give me an example how I can change to "da" and then unpack please?

in the offending macro find the word "data"

delete the t and and one of the remaining a's in that word


to unpack

read in the low byte , store bit 7 value of low byte then bitwise and (&) the low byte with 127
then
read in the high byte left shift it left 1 digit then add 1 to result if bit7 result from low byte is not zero

[Ioannis]

I think Charles should be informed about this.

Ioannis

[wjsmarine]

Thanks Richard,

I'm no aficianado with asm and if needed it sort of defeats the purpose being on a PicBasicPro forum, but it is fast becoming apparent one needs to know some asm if one wants to excel with anything that extends PBP out of its comfort zone and to its boundaries. I'm almost completely ignorant on this subject despite being very willing to learn. I'd like to think those who are very knowledgeable and experienced in such matters would welcome the opportunity to pass on same and offer due guidance - call it a legacy if you will.

I don't want anyone to write my code for me but I'm flying blind under these circumstances and have no clue on such matters.

While I can make the change from data to da I don't have much idea how to perform the unpack - while I am grateful for your help, reading it is difficult to understand. If your suggestion means I have to make the changes for every @ printstr line (a learning curve all of its own) then it will be easier pulling teeth and I'd suggest the whole macro thing is a waste of time for me - easier to stick with 18F's to do the job, which don't seem to show this problem. This being the case I'd jump across to Swordfish and dispense with PBP completely but I like to use the 12F and 16F series - and don't like being forced into doing anything I don't want to!

Charles, following on from Ioannis' prompt, if you can shed some light here, please do so. Apart from Richard there is very little input seen to date. If it is all too difficult then I'll move away from these Nokia displays to something more accommodating.

Kind regards,
Bill

[richard]

Charles, following on from Ioannis' prompt, if you can shed some light here, please do so. Apart from Richard there is very little input seen to date. If it is all too difficult then I'll move away from these Nokia displays to something more accommodating.


there is no pbp error here . the asm "data" directive in inappropriate for core 14 devices


I don't want anyone to write my code for me but I'm flying blind under these circumstances and have no clue on such matters

what part of my description can't you not follow , there is no need for asm at all to do this.


hint


packedbit var bit
StringOut:
Readcode Addr, Char ' Get a character would be the low byte
packedbit= char.7
char=char&127
.......

[wjsmarine]

Hi Richard,

Thanks for the help. I started work Friday so my time on the forum will be a lot less from now on unfortunately - but good that I have an income now after so long.

Can I ask: have you tried these changes yourself and seen the results on a Nokia 3310 lcd? I tried your suggestion per my interpretation below and it compiled/assembled without errors but the result was the same as if the changes were not made - so I'm assuming I didn't do it correctly or am still missing something.

I made the following edits after replacing data with da:



'for @ printstr
Addr VAR WORD
'Char VAR byte ' lcdchardata[0] [WJS] as it was
Char VAR word ' lcdchardata[0] [WJS] now for modification

StringOut:
' Edits for 16F devices.
Readcode Addr, Char ' Get a character
if Char = 0 then StringDone ' Look for Null char, Stop if found
packedbit = char.7 ' Store bit 7 value of low byte
char.lowbyte = char.lowbyte&127 ' Bitwise And (&) the low byte with 127

Char.highbyte = (Char.highbyte <<1) ' left shift the high byte 1 digit (multiplying by 2n)
if packedbit <> 0 then
Char.highbyte = Char.highbyte+1
endif

Gosub Lcd_GotoXY
Lcd_Data = Char

gosub Lcd_SendChar ' puttext
Addr = Addr + 1 ' Point to next character
PosX = PosX + 6
if PosX > 78 then ' If end of line, cycle to next line
PosY = PosY + 1
PosX = 0
endif
goto StringOut ' Continue with rest of the string

'' Original
' Readcode Addr, Char ' Get a character
' if Char = 0 then StringDone ' Look for Null char, Stop if found
' Gosub Lcd_GotoXY
' Lcd_Data = Char

' gosub Lcd_SendChar ' puttext
' Addr = Addr + 1 ' Point to next character
' PosX = PosX + 6
' if PosX > 78 then ' If end of line, cycle to next line
' PosY = PosY + 1
' PosX = 0
' endif
' goto StringOut ' Continue with rest of the string

StringDone:
return



Please show me where have I blundered.

Kind regards,
Bill

[richard]

I don't have a nokia display so I can't really try any of this.

you have not shown how u modified the macro


'for @ printstr
Addr VAR WORD
'Char VAR byte ' lcdchardata[0] [WJS] as it was
Char VAR word ' lcdchardata[0] [WJS] now for modification

StringOut:
' Edits for 16F devices.
Readcode Addr, Char ' Get a character



if Char = 0 then StringDone ' Look for Null char, Stop if found
will not work as intended since either the high byte or the low byte could be a null not necessarily both
packedbit = char.7 ' Store bit 7 value of low byte
char.lowbyte = char.lowbyte&127 ' Bitwise And (&) the low byte with 127

Char.highbyte = (Char.highbyte <<1) ' left shift the high byte 1 digit (multiplying by 2n)
if packedbit <> 0 then
Char.highbyte = Char.highbyte+1

endif

Gosub Lcd_GotoXY
Lcd_Data = Char

gosub Lcd_SendChar ' puttext
Addr = Addr + 1 ' Point to next character
the address would now need to incremented by 2
and when did you send the highbyte to the screen ?
PosX = PosX + 6
if PosX > 78 then ' If end of line, cycle to next line
PosY = PosY + 1
PosX = 0
endif
goto StringOut ' Continue with rest of the string


if codespace is a big issue then

Lcd_SendChar:
lookdown Lcd_data,_
[" !\#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~"],CharNum
Lcd_data = CharNum +32

wastes 117 words of code space for no appreciable reason



SELECT CASE lcd_data
Case 32
FC(0)=$00:FC(1)=$00:FC(2)=$00:FC(3)=$00:FC(4)=$00 ' // sp
Case 33
FC(0)=$00:FC(1)=$00:FC(2)=$2f:FC(3)=$00:FC(4)=$00 ' // !
Case 34
FC(0)=$00:FC(1)=$07:FC(2)=$00:FC(3)=$07:FC(4)=$00 ' // " (shown as \ in Lookdown table)
.......................

manages to use 1662 words of code space to store 235 words of data, it could be far more efficient

see http://www.picbasic.co.uk/forum/showthread.php?t=23758

where I use a 7x8 font that is stored as a packed 14bit array for pic16 chips , the posted code has routines to unpack the font

[wjsmarine]

Hi Richard,

I've studied your hints and hacked away all day with no success, I've concluded it's beyond me so will give up on this idea. I spent a lot of time looking at your ssd1306 code also and suspect it could be modified to use on the 84x48 pixel Nokia display, but that too is over my head. I have fully functioning code working on an 18F so will use it instead, I just have to manufacture another pcb to scale up from dip18 to dip28, kicking myself for not doing that in the first place.

I'm not wasting more time on this because it is overtaken by Mister_e's code, which I mentioned earlier works flawlessly on a F88, but has similar issues with a 16F1847 as this thread. If you haven't done already please take a look at the "Using Nokia LCD" post - if it can be made to work on this chip it opens the door to a lot more projects. Over 200,000 views on this thread shows there are a lot of folks interested in these cheap graphic displays so I think it is worth doing.

PM me your address and I'll post a display module with 16F1847 to you for your use as a small token of appreciation for your ongoing contributions, you should get it next week.

Kind regards,
Bill

[richard]

I'll post a display module with 16F1847 to you for your use as a small token of appreciation for your ongoing contributions, you should get it next week.
A generous offer , when I get it I will see if I can port my ssd1306 code to that display.
it does not look too difficult [famous last words]

[wjsmarine]

No problem, I just packed it in a small padded Jiffy bag and will post tomorrow - check your PM.

Did you take a look at Mister_e's work on the "Using Nokia LCD" by chance? You sound like you are the man to take it from its 99% to that last 1% (and, fingers crossed, able to work with the 16F1847). Steve did a great job but I fear he is no longer around such is the deafening silence, hope I'm wrong.

Kind regards,
Bill

[richard]

Did you take a look at Mister_e's work on the "Using Nokia LCD" by chance?

I have looked but its chief issue for me is the way the font is stored/retrieved ,for a pic16 with limited resources
its just too inefficient. if it ports to the method I used for the ssd1306 controller then it will be a better result.
not only that imho usercommand [a sadly underused feature ] makes nicer code