GPUtils PIC bug?

[n_K]

So I've been using GPUtils for a few weeks now, used to use MPASM back in the day on windows but I have to say I really like GPUtils.

So I've been playing around with this PIC 18F4520 IC I have and have been writing a pretty large program (in terms of pages). I've been frustrated for a whole day now on why a function won't work and has undesirable behaviour - returns 0 or crashes or resets the PIC... Then I realised something - the code was too long for one page and I've been coding in the next page without knowing it.

So (this is all reloctable code - NOT absolute) I read up on using BANKSEL and PAGESEL in GPUtils... Unfortunately, it appears to do buggar all, I'm not sure if I'm using it wrong or if features is bugged and just isn't working. This is the code I'm using (from the lst to see the addresses):

000288   0b0f	 andlw   0xf			  ANDLW b'00001111'
										   PAGESEL 400h
										   PAGESEL FUNC
00028a   ec00	 call    0x400, 0		 CALL FUNC
00028c   f002
										   PAGESEL $
00028e   0100	 movlb   0			    BANKSEL d2
.....
										   org 0x400

000400   010f	 movlb   0xf			  FUNC BANKSEL PCL
										   PAGESEL $
000402   6e02	 movwf   0x2, 0		   movwf d1
000404   0e04	 movlw   0x4			  movlw b'00000100'

And as can be seen - PAGESEL is doing nothing if I provide it with the function name or an address, I have to alter the PCLATH register manually as if I'm using absolute assembly which is a bit dumb and will always vary depending on what's added or what it's linked with or if anything is changed so I'm unsure if I'm doing something wrong or if I need to report a bug.

(Compiling using: gpasm -c 18f4520LCD.asm, linking using: gplink -m -c -s /usr/share/gputils/lkr/18f4520.lkr -o 18f4520LCD 18f4520LCD.o)

Anyone else got GPUtils, a programmer and can see if this is happening when they try this?

Thanks!

[Karl L.]

I'm much more familiar with the PIC 18F242 than the 18F4520, but I assume their instruction set is similar. It looks like you are completely disregarding the BSR (Bank Select Register) in your code. You are setting it with instructions such as movlb 0xf, then disregarding that value with movwf 0x2, 0. According to the PICmicro Quick Reference Chart:

RAM access bit

a = 0: RAM location in Access RAM (BSR register is ignored)

a = 1: RAM bank is specified by BSR register (default)

Hex: 6Ff*

Mnemonic: MOVWF f,a

Description: Move WREG to f

Function: WREG ? f

Just out of curiosity, why did you choose to program this project in assembly instead of C? SDCC is a good FOSS C compiler for the PIC18 that is undergoing rapid development. (Check out the SDCC SVN repo if you're interested.) If you would rather have a commercial compiler, the HI-TECH C compiler for the PIC18 is also excellent.

[n_K]

Actually, that is the assembler, there is no movlb in the actual code file

PAGESEL 800h
CALL FUNC
PAGESEL $
BANKSEL d2
movwf d2
....
org 0x400
FUNC BANKSEL PCL
PAGESEL $
movwf d1
movlw b'00000100'

So quite what is going on... *shrug* I've not a clue.

Never been a fan of C on PICs, I like to know what they're actually doing... Or attempt, found some I2C 24c16 that reads/programs fine with a JDM programmer but using my own I2C implementations and loads off the 'net using custom methods and using the actual in-built I2C commands - None worked! There's a lot of I2C C examples for PIC chips on the net that consist of something like

I2CStart();

I2CRead('1010000');

I2CWrite();

...Fair enough you can write fast code but what the hell is actually going on? Same as on a PICAXE, well, I find PICAXE worse for it's got a bootloader running, all the time, so anything that's timing critical or to be very optimised will never be optimised.

EDIT: Just seen that SDCC project supports HC08, got an HC11 which appears to be backward compatible that I was slowly going to move on to, might give it a blast on that to see if/how it works :D

Edited January 31, 2013 by n_K

[Karl L.]

C is very advantageous because it allows you to program the controller faster. I agree that its certainly possible to program the PIC18 in C and not understand what its doing underneath, but just because you program it in C doesn't necessarily mean that you don't! Knowing the instruction set means you understand that 16-bit and signed variables have higher overhead, float is not supported in hardware so it should be used sparingly (if at all), and there are only three pointer registers. That's why its probably best to learn assembly first, but I wouldn't rely on it for anything super complicated.

On a side note, I have working I2C code that I wrote to communicate between the PIC18 and a serial EEPROM. (In C, of course! I'm just insane, not a masochist.) It uses the PIC18's built-in I2C functions and relies on no third-party code. I will send it to you if you're interested.

Actually, that is the assembler, there is no movlb in the actual code file

Maybe you should consider invoking movlb and movwf directly then. Even if you are not writing those commands at the moment, it is happening according to the code you posted.

Also, doesn't using directives that expand into multiple assembly instructions somewhat defeat the reason why you're using assembly instead of C?

[n_K]

This is the problem though, because it's relocatable code, if I mix another library with it or even if I don't, it's a mis-match of things and so unless I specifically put the address in - I don't know where it'll be nor what bits to set when using one function from another. I only spotted why this was going wrong by accident in gpsim, seeing it be fine at address 400 then come to an increment of PCL and suddenly jump to instruction 6 out of the blue - then saw the PCLATH and PCLATU registers - set them manually and it works fine but I shouldn't have to set them manually - gpasm and gplink should be doing that with the pagesel/banksel instructions as far as I'm aware, so I'm not sure if I'm doing it wrong or if the program is bugged and I'd feel a complete tool to sign up to sourceforge to report this problem if it isn't a bug :(. [There was also no RTL/RRL instruction present, for some reason it's 'rrcf' and 'rlcf' :s again not sure if it's a bug]

I've tried using the internal I2C functions too but this EEPROM (there's no spec sheet either for this exact model, 24c16 (Not 16A! And not atmel!) by and the 18f4520 just don't like each other for some reason. I've got a 16f88 I might try with it sometime but I also found a 93c46n that works brilliantly and prefer having seperate SDO and SDI lines :D

[Karl L.]

I've tried using the internal I2C functions too but this EEPROM (there's no spec sheet either for this exact model, 24c16 (Not 16A! And not atmel!) by and the 18f4520 just don't like each other for some reason. I've got a 16f88 I might try with it sometime but I also found a 93c46n that works brilliantly and prefer having seperate SDO and SDI lines :D

I have the 24LC515. There are a couple important considerations, but do you have 10K pull-up resistors on your SCL and SDA lines? Also, are you sure you have the address right? On my EEPROM, the A0, A1, and A2 pins determine the user-defined part of the address, which must be included with the static device address. If you don't have the datasheet and don't know the device address, you could always try brute-forcing it by trying every address until one is accepted. (Since its only 8 bits that shouldn't take very long.) You would need to be sure everything else was correct for that to actually work, though.

[n_K]

10k!? No! I've tried 3.8K and 2.2K.

The address is the problem, well, might be... The atmel 16a spec sheet says it has no address pins and it's forced to 000 but I grounded them anyway.

The first time I tried just doing my own 'brute force' attempty at the whole I2C thing and copied atmel's waveforms (not using the in-built I2C lines) and it did kind of work, and by kind-of I mean sporatically (it was not running at 100KHz, it was running using LEDs so I could see it, so very slow). If I powered the device on 20 times, say, 5 times the EEPROM would respond with the data, some of those times it would be delayed by a few clocks, but the other 15 or so times it wouldn't do anything, but the JDM programmer for the PC reads the chip fine each time!

EDIT: Or would it? Might be getting confused between the 93c46n and the 24c16 here actually as to if it even replied properly. Don't think it did actually.

[Karl L.]

The datasheet for my EEPROM says the I2C speed should be between 100 - 400 KHz, so I set mine to 350 KHz. Like I already mentioned, I put a 10K pull-up resistor on the SCL and SDA lines. I used 0xA0 as my address because my static device ID is 1010, my user-configurable ID pins are all grounded (000), and I want to write by default (0). Since the PIC18 is big-endian, my 8-bit address is 10100000 (0xA0) when I want to write and 10100001 (0xA1) when I want to read. Obviously it will be a little different depending on the EEPROM chip you're using, but maybe my configuration will help.

[n_K]

Here's one attempt I tried which literally just hammered the data out at a slow speed (so I could see the LEDs):

;RB0 = CLK
;RB1 = OUT/IN
;START
movlw   b'00000011'
movwf   LATB
call Delay
movlw   b'00000001'
movwf   LATB
call Delay
movlw   b'00000000'
movwf   LATB
call Delay
;/START
;DATA
;101000010
movlw   b'00000011'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000001'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000011'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000001'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000001'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000001'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000001'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000011'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
movlw   b'00000001'
movwf   LATB
call Delay
;
  movlw   b'00000000'
  movwf   LATB
  call Delay
;
;/DATA
bsf LATD, 0
call Delay
call Delay

;CHECK
movlw   b'00000010' ;RB1 now an input
movwf   TRISB
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
bsf	 LATB, 0
call Delay
bcf	 LATB, 0
call Delay
;
;/CHECK
bcf LATD, 0

;STOP
movlw   b'00000001'
movwf   LATB
call Delay
movlw   b'00000011'
movwf   LATB
call Delay
movlw   b'00000000'
movwf   LATB
call Delay
;/STOP

goto Start

Another one I quickly attempted using the actual PIC I2C functions:

movlw b'00000000'
movwf INTCON
movwf INTCON2
movwf INTCON3
movwf ADCON0
movlw b'00001111'
movwf ADCON1
movwf CMCON
movlw b'00000000'
movwf RCON
movwf PIR1
movwf PIR2
movwf PIE1
movwf PIE2
movwf IPR1
movwf IPR2
movwf CMCON
movwf CVRCON
movlw b'00000000'
movwf SSPADD
movlw b'10000000'
movwf SSPSTAT
movlw b'00001000'

movwf SSPCON1
movlw b'00000000'
movwf SSPCON2
call Delay
BSF SSPCON1, 5
call Delay
BSF PIR1, 3
call Delay
BSF SSPCON2, 0
BSF PIR1, 3
call Delay
;movlw b'11010000'
movlw b'110100000'
movwf SSPBUF
BCF SSPCON2, 6
call Delay
call Delay
movf SSPBUF, w
movwf LATA
BSF SSPCON2, 6
call DelayL
movlw b'110100000'
movwf SSPBUF
BCF SSPCON2, 6
call Delay
call Delay
swapf SSPBUF, w
movwf LATA
sleep

Now I look I've not a clue about the 1101... movlw, it's got 9 bits :s. And the other one I tried was just basically nicked from http://nnp.ucsd.edu/phy120B/0_pic_asm_files/i2c_CFD.asm though I changed it to remove the 2nd lot of addressing as the 24c256 has 16-bit addresses that the 24c16 and below don't.

But yes unfortunately none worked. I might have another go at them in the future.... As I'm currently having problems with pagesel in gputils, do you know if there's an official MPASM for linux anywhere?

[Karl L.]

I don't think MPASM was ever released for Linux. However, there is GPL source code available for the MPLAB Assembler and MPLAB C compiler for the PIC24, dsPIC, and PIC32. The existing PIC16 and PIC18 toolchains are completely incompatible with the PIC24 and PIC32 toolchains, so it might not do you any good.

In case you are interested, the C30 source code is buried on a page deep in Microchip's website that looks like something straight out of 1990. There is also some discussion of building an older version of the compiler for Debian on the Microchip user forums. The Debianized source is quite old and not in the modern Debian packaging format, but the debian/control and debian/patches/* should get you started if you want to build the current version of C30 for Debian or some other Linux distribution. Good luck!

Edit: I just discovered something. If you install the free (but not FOSS) MPLABX in Linux, it will install MPASM. MPLABX 1.60 installed MPASM 5.48 as /opt/microchip/mplabx/mpasmx/mpasmx on my machine. Potentially unlike Microchip's ASM30, the version of MPASM installed with MPLABX will work with the PIC18.

[n_K]

That, is, insane.

Warning[205]: Found directive in column 1. (PAGESEL)

Message[312]: Page or Bank selection not needed for this device. No code generated.

So... What the hell? Because I've got an 18f4520 I need to manage the PCLATH and PCLATU registers myself? What a heap of ****. I can't believe how pathetic that is! What's the point in having the functions at all then if you can't use them.

[n_K]

So I got a DSPIC30F4013 in the mail today... Thought it'd be fun to play around with it in gputils but there appears to be a slight minor drawback; it's not supported in gputils :(.

So I just had a quick look around and saw the HI TECH C compiler that supports it for $850 and some 'csd 24 bit' that supports it for $350... Both are $850 and $350 too expensive for me for playing around with!

Got any suggestions for C compilers ? :p Or heck, even assembly at this point!

[Karl L.]

I don't have a dsPIC, but from what I understand they are approximately equivalent to the PIC24. I use Microchip's MPBLAB XC compiler with my PIC24FJ128GA010. I think that will work with your DSPIC30F4013 as well.

[n_K]

Alright nice I'll give it a go when the PC I do PIC work on is back in operation.

[Karl L.]

I just checked the XC16 v1.1 User Manual to make sure that my assumption that the compiler supports your device as well is correct. According to section 1.2:

1.2 DEVICE DESCRIPTION

The MPLAB XC16 C compiler fully supports all Microchip 16-bit devices:

? The dsPIC? family of digital signal controllers combines the high performance required in digital signal processor (DSP) applications with standard microcontroller (MCU) features needed for embedded applications.

? The PIC24 family of MCUs are identical to the dsPIC DSCs with the exception that they do not have the digital signal controller module or that subset of instructions. They are a subset, and are high-performance MCUs intended for applications that do not require the power of the DSC capabilities.

The latest version of the MPLAB XC 16 compiler and user manual can be downloaded here, and the source code can be downloaded here (since the compiler is GCC based). The XC16 C compiler runs perfectly on my Debian 7 (AMD64) installation, so I assume it will work equally as well for you on Arch.

[n_K]

Humm, bit of a problem;

/usr//bin/sdcc -mpic16 -V -p18f4520 -DDEBUG --denable-peeps --opt-code-size --optimize-cmp --optimize-df -I. -c i2c_master.c

+ /usr//bin/sdcpp -nostdinc -Wall -DDEBUG -I. -Dpic18f4520 -D__18f4520 -D__SDCC_PIC18F4520 -DSTACK_MODEL_SMALL -D__STACK_MODEL_SMALL -obj-ext=.o -D__SDCC=3_2_1 -DSDCC=321 -D__SDCC_REVISION=8413 -DSDCC_REVISION=8413 -D__SDCC_pic16 -DSDCC_pic16 -D__pic16 -D__STDC_NO_COMPLEX__ -D__STDC_NO_THREADS__ -D__STDC_NO_ATOMICS__ -D__STDC_NO_VLA__ -isystem /usr//bin/../share/sdcc/include/pic16 -isystem /usr/share/sdcc/include/pic16 -isystem /usr//bin/../share/sdcc/include -isystem /usr/share/sdcc/include i2c_master.c

In file included from i2c_master.h:4,

from i2c_master.c:1:

/usr//bin/../share/sdcc/include/pic16/pic18fregs.h:580:26: error: pic18f4520.h: No such file or directory

And the files in that directory;

adc.h delay.h float.h i2c.h malloc.h p18fxxx.inc pic18fregs.h signal.h stddef.h stdio.h string.h

ctype.h errno.h gstack.h limits.h math.h pic16devices.txt sdcc-lib.h stdarg.h stdint.h stdlib.h usart.h

I edited the PKGBUILD and compiled my own SVN version of sdcc but I'm not sure what I'm doing wrong that the files aren't there!

EDIT: Scratch that, downloaded the file off the net and it works, not sure why it isn't included by default though.

New problem! config.h:9: syntax error: token -> 'char' ; column 9

make: *** [main.o] Error 1

#define VERSION 0.1

// Set the __CONFIG words:

code char at __CONFIG1H _conf1 = _OSC_INTIO67_1H

code char at __CONFIG2L _conf2 = _VREGEN_ON_2L & _PUT_ON_2L & _BODEN_OFF_2L;

^ It doesn't seem to like that... And I haven't got a clue what it SHOULD look like :p

Edited February 6, 2013 by n_K

[Karl L.]

You should set the device configuration using the pragmas documented in the XC16 C compiler manual.

2.5.14 Specifying Configuration Bits

The #pragma config directive may be used to program the configuration bits for a device. The pragma has the form:

#pragma config setting = state|value

#pragma config register = value

where setting is a configuration setting descriptor (e.g., WDT), state is a descriptive value (e.g., ON) and value is a numerical value. The register token may represent a whole configuration word register, e.g., CONFIG1L. Use the native keywords discussed in the Differences section to look up information on the semantics of this directive.

2.5.14.1 EXAMPLE

The following shows configuration bits being specified using this pragma.

#pragma config WDT=ON, WDTPS = 0x1A

If you need to use special function registers - which I'm guessing your probably do - read chapter 4 of the manual. In particular, the following is an excerpt from section 4.6 "using SFRs":

The convention in the processor header files is that each SFR is named, using the same name that appears in the data sheet for the part ? for example, CORCON for the Core Control register. If the register has individual bits that might be of interest, then there will also be a structure defined for that SFR, and the name of the structure will be the same as the SFR name, with ?bits? appended. For example, CORCONbits for the Core Control register. The individual bits (or bit fields) are named in the structure using the names in the data sheet ? for example PSV for the PSV bit of the CORCON register.

I recommend that you have both the datasheet and compiler manual handy until you know your microcontroller and compiler's special features fairly well. My approach is usually to read the datasheet first to learn the neat features specific to my microcontroller, then read the compiler manual so I understand the caveats and best practices recommended by its authors (who presumably know more about the architecture than me). However, if you have to pick just one section of the datasheet or compiler manual to read, "Chapter 2. Common C Interface" should absolutely be it. That chapter will explain enough to get you started, at least.

[n_K]

This is sdcc not the official MC compiler.

I can't understand why it won't work, this is using an example from the sdcc forums that another user said works fine on the PIC I'm trying to compile for.

main.c:11: warning 115: unknown or unsupported #pragma directive '__CONFIG1H = _OSC_HS_PLL_1H & _OSCS_ON_1H;' (when it was #pragma __CONFIG1H = _OSC_HS_PLL_1H & _OSCS_ON_1H;)

main.c:11: warning 191: #pragma config: bad argument(s); pragma ignored (when it was #pragma config __CONFIG1H = _OSC_HS_PLL_1H & _OSCS_ON_1H;)

I got the config line I changed to #pragma from another post on their forums :s

[Karl L.]

Does SDCC support the dsPIC? I though it only supported the PIC18.

I have only used the XC16 C compiler with my PIC24, but I have used both the HI-TECH C compiler and SDCC with my PIC18. I can try to find my makefile and some basic source code if you are interested. All I could find off-hand, though, is the configuration I have often used for the PIC18 with HI-TECH C.

#ifndef CONFIG_H
#define CONFIG_H

#include <htc.h>

/* PIC18F242 configuration details */
#pragma config OSC=HSPLL
#pragma config BOR=OFF
#pragma config PWRT=OFF
#pragma config WDT=OFF
#pragma config DEBUG=OFF
#pragma config LVP=OFF

/* Bit manipulation macros suggested by the HI-TECH C manual */
#define bitset(var,bitno) ((var) |= (1 << (bitno)))
#define bitclr(var,bitno) ((var) &= ~(1 << (bitno)))
#define bittst(var,bitno) (var & (1 << (bitno)))

#endif // CONFIG_H
[/CODE]

[b]Edit:[/b] OK. I found it. I can't vouch for the completeness of this code and I make no claims that it represents good style or practice, but it [i]does[/i] compile on SDCC 3.2 and run on the PIC18F242. The zip archive with the code is attached to this post.

SDCC_PIC18_SERIALCALC_EX.zip

[n_K]

I was compiling and testing for the 18F4520 :p Also got an 18F4550 too now.

I'd like to try out the dspic but unfortunately either the hardware and/or the software fails half way through reading/programming them so I'm actually unable to do anything with them.

[n_K]

Right I've got the XC8 free edition and MPLABX installed on this PC, tried compiling the 18F4520 example for an LCD I got and it does compile... Only problem is that it's not starting instructions at 0x0000 and is instead starting at 0x1067... I can't really understand why?

http://www.wvshare.c...-Touch-LCD-A.7z is the code and I converted it from an MPLAB 8.x project, any ideas?

"invalid address 0xf0010f, possibly not hex file for correct pic type?"

"picprog --device=pic18f4520 --erase --burn -i "/tmp/TOUCH.X.production.hex" --jdm --pic /dev/ttyS0

Picprog version 1.9.1, Copyright ? 2010 Jaakko Hyv?tti <[email protected]>

Picprog comes with ABSOLUTELY NO WARRANTY; for details

type `picprog --warranty'. This is free software,

and you are welcome to redistribute it under certain conditions;

type `picprog --copying' for details.

Trying realtime priority 1

Bound to CPU 0

Bound to CPU 1

Bound to CPU 2

Using >1 ?s delays. --rdtsc may work for faster timings.

/tmp/TOUCH.X.production.hex:397:invalid input line."

Line 397 is in bold (lines 395-end):

":107FE80000000000000000000000000057617665F6

:087FF80053686172650000008E

:020000040020DA

:08000000FFFFFFFFFFFFFFFF00

:020000040030CA

:0E000000FF071F1FFF8385FF0FC00FE00F409B

:00000001FF"

Edited March 6, 2013 by n_K

[Karl L.]

Microcontroller code compiled with an ANSI C compiler never starts at address 0. It uses the reserved space before main() for global and static variables.

Unfortunately I can't look at your code because it doesn't extract properly for me. However, this adafruit LCD tutorial is a good place to start. I also attached my code for a basic calculator using a 4 line 20 character LCD and 4x4 keypad. It compiles using the HI-TECH C compiler (and most likely the XC8 compiler as well, although I haven't tested that). I uploaded it to my PIC18F242 from MPLABX using the PICkit 2.

LCDCalc.zip

[n_K]

Right I've found the problem, apparently it's because MPLAB is (crap) writing 'odd byte counts' to save space or something, and someone wrote a program (fixhex3) to fix the problems but appears it's a private release or something.

So I have absolutley no idea how to make MPLABX create proper files or how to change invalid files into proper ones either :s

[n_K]

Right I got it programmed, having to use picpgm as picprog doesn't support the 'odd byte count' programming type by picpgm does, the downside to using picpgm is that it takes absolutely ages to write, and verifying... WOW... I've got it not verifying as it took over 30 second to verify :(.

But I've got an incredibly strange action from this code, I've tried a 3.4Mhz oscillator, 8Mhz oscillator and 20Mhz oscillator - they all seem to run the code at the same speed, even tried reducing the values in the loop functions - and the program doesn't speed up AT ALL, it is absolutely crazy :s.

[n_K]

Wow, I can't actually believe it :o.

Got an 18LF4520 reading data from an RTC using C, and, it works!

Unbelievable! (Well, I've got it setting LEDs depending on what the seconds are) but heck it's astonishing to be able to write that in 20 lines of code instead of 300 or so, though it does seem to be compiling it to inefficient code;

Program space used 348h ( 840) of 8000h bytes ( 2.6%)

Data space used 15h ( 21) of 600h bytes ( 1.4%)

Ah well, that's nifty!

[n_K]

Right actually it's not working... And I've no idea why! It's getting back straight 1s.

Here's the code I kinda 'converted' it from;

  RTC_t rtc;
  I2C_start();    // START
  I2C_out_byte(0xD0);   // DS1307 hardware address + write command
  I2C_sack();	 // slave ACK
  I2C_out_byte(0x00);   // we will read from this address
  I2C_sack();	 // slave ACK
  I2C_start();	 // repeated START
  I2C_out_byte(0xD1);   // DS1307 hardware address + read command
  I2C_sack();	 // slave ACK
  rtc.sec = I2C_in_byte();   // read sec
  I2C_ack();	 // master ACK
  rtc.min = I2C_in_byte();   // read min
  I2C_ack();	 // master ACK
  rtc.hour = I2C_in_byte();  // read hour
  I2C_ack();	 // master ACK
  rtc.dweek = I2C_in_byte();  // read dweek
  I2C_ack();	 // master ACK
  rtc.day = I2C_in_byte();  // read day
  I2C_ack();	 // master ACK
  rtc.month = I2C_in_byte();  // read month
  I2C_ack();	 // master ACK
  rtc.year = I2C_in_byte();  // read year
  I2C_nack();	 // master NACK

And here's my code;

#define I2C_V2 true
#include <plib/i2c.h>
    int Data[6];
    OSCCON = 0b10100011;
    TRISC = 0x00; // turn on tri-state register and
				 // make all output pins
    PORTC = 0x00; // make all output pins LOW
    TRISA = 0x00;
    PORTA = 0xFF;
    OpenI2C( MASTER, SLEW_OFF);
    __delay_ms(90);
    __delay_ms(90);
    __delay_ms(90);
    PORTA = 0x00;
    SSPADD = 0x3F;
    StartI2C();
    IdleI2C();
    WriteI2C( 0xD0 );  // sends address to the
    IdleI2C();
    WriteI2C( 0x00 );
    IdleI2C();
    StopI2C();

    StartI2C();
    IdleI2C();
    WriteI2C( 0xD1 );  // sends address to the
    IdleI2C();
    WriteI2C( 0x00 );  // sends address to the
    IdleI2C();
//    RestartI2C();
    Data[0] = ReadI2C();
    IdleI2C();
    AckI2C();
    IdleI2C();
    Data[1] = ReadI2C();
    IdleI2C();
    AckI2C();
    IdleI2C();
    Data[2] = ReadI2C();
    IdleI2C();
    AckI2C();
    IdleI2C();
    Data[3] = ReadI2C();
    IdleI2C();
    AckI2C();
    IdleI2C();
    Data[4] = ReadI2C();
    IdleI2C();
    AckI2C();
    IdleI2C();
    Data[5] = ReadI2C();
    IdleI2C();
    AckI2C();
    IdleI2C();
    Data[6] = ReadI2C();
    IdleI2C();
    NotAckI2C();
    IdleI2C();
    StopI2C();
    PORTA = Data[0];
    __delay_ms(90);
    __delay_ms(90);
    __delay_ms(90);
    __delay_ms(90);
    PORTA = 0;
    __delay_ms(90);
    __delay_ms(90);
    __delay_ms(90);
    __delay_ms(90);
    PORTA = Data[1];
    Sleep();

Any ideas?