Code:
ADCON0 |= Channel<<2; // Clear Channel selection bits
This doesn't clear the current channel select bits before ORing in the new ones.
In your simple single channel example it works, but it won't as soon as you try to change the channel.
Code:
return (int)(ADRESH<<8)+ ADRESL ; // Return 10 bit ADC value
Due to C's integer promotion rules, the cast isn't required but if you do cast it should be 'unsigned int'.
It's also usually more efficient to OR instead of adding.
Code:
ANSELA = 0x00;
ANSELC = 0x00; // Clear Pin selection bits
if(Channel < 4)
ANSELA |= Channel; // Select Channel
else
ANSELC |= Channel;
These are just flat out wrong. ANSELx are bit-oriented, so something like 'ANSELC |= 7' doesn't set bit 7,
it sets bits 2-0. It also clears any existing ANSELx setting, changing pins back to digital mode.
Code:
CM1CON0 = 0x07; // Shut off the Comparator, so that pins are available for ADC
FVRCON = 0x00; // Shut off the Voltage Reference for Comparator
The first one doesn't do what it says it does, and the second is not needed.
Anyway, the comparators don't prevent the ADC from working (unless you enable the comparator output to an ADC pin).
That sets RC3 as input, but the OP wants to "use 2 adc pins from 2 different ports (RA4, RC3)",
so RA4 wasn't addressed. It would work since all pins are inputs by default, but I like to explicitly set them,
especially if you're trying to show someone what to do.
The other things are minor (mixing signed and unsigned ints, placing config settings after includes, etc).
EDIT: and personally, I'd move the setting of the pin mode + TRIS out of the InitADC function.