Can someone explain the meaning of PORTC?

What I expect is that I am wiring up a network device to a twisted pair to read and write packets which will be expressed as differential voltages. The protocol is Toyota AVC-Lan which is similar to IEBus. The convention is for these terminals to be labeled TX+ for the high voltage and TX- for the low voltage.

I don't understand the code snippet below. What I'm expecting is, I'll understand which Arduino Nano pins to connect the TX+ and TX- terminals of the network.

Although I don't understand this section, I think this would be the place for the configuration I am expecting. Can anybody explain to me what is being done here?

// TX LED connected to Pin1  of  PORTC
#define TX_LED_DDR		DDRC
#define TX_LED_PORT		PORTC
#define	TX_LED_PIN		PINC
#define TX_LED_OUT		2

// RX LED connected to Pin4  of  PORTC
#define RX_LED_DDR		DDRC
#define RX_LED_PORT		PORTC
#define	RX_LED_PIN		PINC
#define RX_LED_OUT		4

The project is this: GitHub - douglasheld2/AVC-LAN-Module-Builder: Firmware for ATMega to Emulate ANY AVC-LAN device

It could be, that I connect the TX wires to the same ports as the resistors (as I think that's the proper termination topology) but that wouldn't explain which one is the high and which one is the low.

Many thanks. OK, so this PORTC stuff is just a poorly self-documenting and error-prone way to very quickly perform actions like pinMode( 6, OUTPUT ); etc.

As the program will be switching between reading and writing on the two pins, I think I can see why the author chose this route.

If I can figure it out and get it working, I will contribute some better comments to this config file.

A port on a processor is a group of (usually byte wide or 8 bits) dedicated to a specific collection of data. These can be internal or external. Timer ports (registers) would be an example of an internal port. Normally port A, B, C would be 8 bit groupings on a microprocessor addressed at the same address. That definition is now very convoluted and like a sea port it just indicates where a certain type of information can be sent or received such as HDMI, VGA, Serial etc and no longer in groups of 8. Yes I still come across ones I do not know about such as IPMI.

No

Port manipulation is a very powerful technique for setting, writing or reading all the lines of that port in one go.
So for example

DDRD = B11111111;   //sets ALL pins D0 - D7 as outputs.
then
PORTD = B11111111; //sets all those outputs as 1

2 lines of code. Error prone - no. Write that your way - 16 lines.

Is your way clearer? lesss error prone?
I'd contend properly (where necessary) commented compact code is clearer and less error prone.

Of course you need to understand about the ports.
Similarly, how self-documenting is this

c = ( c >= 'a' && c <= 'z' ) ? ( c - 32 ) : c;

these defines are references to hardware registers in the Atmel 328p. you can find more info in the datasheet, pg 73

not clear why the code is not using pinMode() or digitRead/Write()

Your c = ... ? ... : c; code looks like a horrible way to uppercase Roman letters :slight_smile: but who knows if I have it right! (That's not from my project is it??)

@johnerrington I suspect you might be able to understand the snippet I pasted. Would it be correct to convert in this way?

// TX LED connected to Pin1 of  PORTC (i.e. Arduino terminal A1)
#define TX_LED_DDR     DDRC
#define TX_LED_PORT    PORTC
#define	TX_LED_PIN     PINC
#define TX_LED_OUT     0b00000010

// RX LED connected to Pin4 of PORTC (i.e. Arduino terminal A4)
#define RX_LED_DDR     DDRC
#define RX_LED_PORT    PORTC
#define	RX_LED_PIN     PINC
#define RX_LED_OUT     0b00000100

I think the danger in how this was written is that if an implementer naively tried to change RX_LED from 4 to 6 in decimal, they would be setting two pins instead of one.

Importantly, the place where this code is used later looks like:

./avclan.ino:179:    cbi(TX_LED_PORT, TX_LED_OUT);
./avclan.ino:181:    cbi(RX_LED_PORT, RX_LED_OUT);
./src/avclan-drv.h:28:#define TX_LED_ON	sbi(TX_LED_PORT, TX_LED_OUT);
./src/avclan-drv.h:29:#define TX_LED_OFF	cbi(TX_LED_PORT, TX_LED_OUT);
./src/avclan-drv.h:30:#define RX_LED_ON	sbi(RX_LED_PORT, RX_LED_OUT);
./src/avclan-drv.h:31:#define RX_LED_OFF	cbi(RX_LED_PORT, RX_LED_OUT);
./src/avclan-drv.cpp:471:		cbi(TX_LED_PORT, TX_LED_OUT);

Question: These cbi() and sbi() functions - are they similarly "fast" and an appropriate use here? As obviously the RX_LED_ON etc. macros are called within loop().

I really appreciate all the feedback I have received here. Thank you.

Maybe it's worth noting, the original library this project was adapted from uses these values like this. I think it is quite a different use case, since multiple pins are being set at once.

#define DATAOUT_DDR		DDRD
#define DATAOUT_PORT	PORTD
#define	DATAOUT_PIN		PIND
#define DATAOUT			7

#define DATAIN_DDR	DDRB
#define DATAIN_PORT	PORTB
#define	DATAIN_PIN	PINB
#define DATAIN		0

#define INPUT_IS_SET (bit_is_set(DATAIN_PIN, DATAIN))
#define INPUT_IS_CLEAR (bit_is_clear(DATAIN_PIN, DATAIN))

For reference the predecessor project is http://www.softservice.com.pl/corolla/avc/sniffer.zip

Oh, you are right. I have carried forward that error into my correction too.

I think also these undocumented cbi() and sbi() functions may actually accept a decimal parameter. Meaning, the original code wasn't incorrect except for the 2!=1 part.

They of course are not undocumented. They what everyone had to use for ATmega processors prior to Arduino's existance.

Any of several different C/C++ methods of representing a parameter may be used for an argument to any function, including the well documented functions sbi() and cbi().

Nothing.
It are just definitions. Nothing doing here.

Major drawback using PORTC is that it is not portable to other micro controllers.

It seems a very strange idea to name a whole port based on a single pin. As here ALL of port C is named based on pin 0b00000010
so eg if all of port D was being used for indicators I'd name it indicatorsDDR etc.

and yes I agree its clearer and safer to use binary for the actions

// TX LED connected to Pin1 of  PORTC (i.e. Arduino terminal A1)
#define TX_LED_DDR     DDRC
#define TX_LED_PORT    PORTC
#define	TX_LED_PIN     PINC
#define TX_LED_OUT     0b00000010

// RX LED connected to Pin4 of PORTC (i.e. Arduino terminal A4)
#define RX_LED_DDR     DDRC
#define RX_LED_PORT    PORTC
#define	RX_LED_PIN     PINC
#define RX_LED_OUT     0b00000100

I'd suggest the mystery doesn't end there. Why waste analog pins on LEDs?

I also have problems understanding the logic of how the differential voltage data pins are used. They are specified as two digital ports, although I believe the voltages are <1V.

Oh and one more thing. I think you and I possibly confused the fourth macro in each set for a bitwise notation. I think you'll find in #post_8 that these values TX_LED_OUT are used as arguments to the cbi() and sbi() functions, which I think takes an integer from 0 to 7, specifying a single bit. So 0b0000'0110 would set/check bit number six, not bits 1 and 2.

So:

  1. It's confusing to put the pin number spec at the bottom of the PORT specification block;
  2. It's possibly confusing to use PORT references for a single pin;
  3. I think my suggestion of making the pin specification in binary is not appropriate for the relevant two lines - it might make it more confusing what is happening.
  4. At the higher abstraction level, it doesn't really make sense to allocate analog ports for LEDs.

I'm all ears if anyone has an observation. I might have got something really wrong here.

If I was forced to dig those up, and type them in without error, I'd depend on the macros:

#define txLedPin  A1
#define TX_LED_DDR portModeRegister(digitalPinToPort(txLedPin))
#define TX_LED_PORT portOutputRegister(digitalPinToPort(txLedPin))
#define TX_LED_PIN portInputRegister(digitalPinToPort(txLedPin))
#define TX_LED_OUT digitalPinToBitMask(txLedPin)

or:

constexpr byte txLedPin = A1;
auto TX_LED_DDR = portModeRegister(digitalPinToPort(txLedPin));
auto TX_LED_PORT = portOutputRegister(digitalPinToPort(txLedPin));
auto TX_LED_PIN = portInputRegister(digitalPinToPort(txLedPin));
auto TX_LED_OUT = digitalPinToBitMask(txLedPin);

...or maybe the library-writer should write the code to use the macros and just take the pin number as a user configuration.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.