3
\$\begingroup\$

I have started to reading up on electronics and embedded software. Lately I have been looking at IC's, particularly shift registers. I thought a good way of learning more about serial communication and shift registers was to implement the internal logic of a shift register.

The one I have chosen is the 74HC595.

I have done this so that it will be similar to working with setting different CPU pins to HIGH or LOW. Imagine that each of the functions manipulating the IC is the value of a CPU pin connected to the IC.

Of course this can be simplified by a easy to use wrapper, so you could just call a function saying which pin to set to LOW or HIGH on the register. But that's not the main point.

ic_example.c:

#include <stdio.h>
#include "../src/ic/ic_74hc595.h"

int main()
{
    struct ic_74hc595 ic = {0, 0, LOW, LOW, LOW};

    /* Initial value check. Should be 0 */
    printf("ic value: %u\n", ic.value);

    /* Set the first bit to 1. */
    ic_set_stcp(&ic, HIGH);
    ic_set_ds(&ic, HIGH);
    ic_set_stcp(&ic, LOW);
    printf("ic value: %u\n", ic.value);

    /* Iterate to third bit */
    ic_set_shcp(&ic, HIGH);
    ic_set_shcp(&ic, LOW);
    ic_set_shcp(&ic, HIGH);
    ic_set_shcp(&ic, LOW);

    /* Set it to 1 */
    ic_set_stcp(&ic, HIGH);
    ic_set_ds(&ic, HIGH);
    ic_set_stcp(&ic, LOW);
    printf("ic value: %u\n", ic.value);

    /* Iterate to last bit */
    ic_set_shcp(&ic, HIGH);
    ic_set_shcp(&ic, LOW);
    ic_set_shcp(&ic, HIGH);
    ic_set_shcp(&ic, LOW);
    ic_set_shcp(&ic, HIGH);
    ic_set_shcp(&ic, LOW);
    ic_set_shcp(&ic, HIGH);
    ic_set_shcp(&ic, LOW);
    ic_set_shcp(&ic, HIGH);
    ic_set_shcp(&ic, LOW);

    /* Set it to 1 */
    ic_set_stcp(&ic, HIGH);
    ic_set_ds(&ic, HIGH);
    ic_set_stcp(&ic, LOW);
    printf("ic value: %u\n", ic.value);

    /* Reset */
    ic_set_mr(&ic, LOW);
    printf("ic value %u\n", ic.value);

    return 0;
}

ic_74hc595.h:

#ifndef IC_74HC595_H_
#define IC_74HC595_H_

#include <stdint.h>

#define HIGH 1
#define LOW 0

typedef _Bool bit;

/**
 * The 74HC595 ic's internal state representation. 
 * 
 * value - The 8-bit value which represents the paralell out data
 * i - The current bit which is iterated to
 * shcp - The shift clock input, which is used to iterate the 8 bits. Each time it switches from LOW (0) to HIGH (1) it iterates one bit. 
 * stcp - The register clock input, which is used to tell the register to accept data. LOW (0) means it doesn't accept data, HIGH (1) means it does accept data. 
 * ds - Serial data, which is used to tell which value the current bit should have; LOW (0) or HIGH (1). 
 */

struct ic_74hc595{
    uint8_t value;
    uint8_t i;
    bit shcp;
    bit stcp;
    bit ds;
};

void ic_set_shcp(struct ic_74hc595 *ic, bit shcp);
void ic_set_stcp(struct ic_74hc595 *ic, bit stcp);
void ic_set_ds(struct ic_74hc595 *ic, bit ds);
void ic_set_mr(struct ic_74hc595 *ic, bit mr);

#endif

ic_74hc959.c:

#include "ic_74hc595.h"

/**
 * Changes the shcp clock to the given value
 *
 * @param ic
 *  The ic to change state and value of
 * 
 * @param shcp 
 *  The shift clock input
 */

void ic_set_shcp(struct ic_74hc595 *ic, bit shcp)
{
    if (ic->shcp == LOW && shcp == HIGH) {
        ic->shcp = HIGH;
        ic->i++;
    }else if (ic->shcp == HIGH && shcp == LOW){
        ic->shcp = LOW;
    }
}

/**
 * Changes the stcp clock to the given value
 *
 * @param ic
 *  The ic to change state and value of
 * 
 * @param stcp
 *  The register clock input
 */

void ic_set_stcp(struct ic_74hc595 *ic, bit stcp)
{
    if (ic->stcp != stcp) {
        ic->stcp = stcp;
    }
}

/**
 * Gives the current bit a new value if stcp is HIGH.
 *
 * @param ic
 *  The ic to change state and value of
 * 
 * @param ds
 *  The data input 
 */

void ic_set_ds(struct ic_74hc595 *ic, bit ds)
{
    if (ic->ds != ds) {
        ic->ds = ds;
    }

    if (ic->stcp == LOW) {
        return;
    }   

    ic->value |= (ds << ic->i); 
}

/**
 * Resets the internal state if mr is LOW.
 *
 * @param ic
 *  The ic to change state and value of
 * 
 * @param mr
 *  The master reset value 
 */

void ic_set_mr(struct ic_74hc595 *ic, bit mr)
{
    if (mr == LOW) {
        ic->value = 0;
        ic->i = 0;
        ic->shcp = LOW;
        ic->stcp = LOW;
        ic->ds = LOW;
    }
}

Compilation command is used:

gcc ./examples/ic_simulator.c ./src/ic/ic_74hc595.c -o ic_simulator -Wall -pedantic

What I would like feedback on:

  • The implementation of the IC's logic. Is it correct?
  • The state of my code. Is it readable? Am I implementing the bit type the right way? Etc.
\$\endgroup\$
  • 1
    \$\begingroup\$ Pretty general question but is : void setVal (Object foo, Value bar) { if (foo->value != bar) foo->value = bar; } really better than : void setVal (Object foo, Value bar) { foo->value = bar; } ? I don't see any real point in your case. \$\endgroup\$ – Josay Jan 10 '12 at 15:57
  • \$\begingroup\$ You are totally right. I missed it when refactoring from another solution I had from the beginning. Setting the value straight away instead will also produce less inustructions, thus be faster. \$\endgroup\$ – rzetterberg Jan 10 '12 at 16:36
  • \$\begingroup\$ Also, you can make the logic of ic_set_shcp simpler : if (ic->shcp != shcp){ ic->shcp = shcp; if (shcp == HIGH) {ic->i++;} } But I'm not sure it's its worth adding an answer for that. \$\endgroup\$ – Josay Jan 10 '12 at 17:49
  • \$\begingroup\$ "CPU pins"? You mean pins on the shift-register package? \$\endgroup\$ – JDługosz Oct 3 '15 at 9:37
4
\$\begingroup\$

OK. Looking at this as a code maintainer and not understanding what the underlying logic of the chip does (which is what will happen in real life).

The function names look a bit generic.
You may want to prefix them with the name of the chip (C unlink C++ does not provide function overloading). So that you can provide similar functions to a set of chips.

From reading the docs this looks good (though I must admit I am not 100% sure).
But a general maintainer is going to look and wonder why half the code paths do nothing.

void ic_set_shcp(struct ic_74hc595 *ic, bit shcp)
{
    if (ic->shcp == LOW && shcp == HIGH) {
        ic->shcp = HIGH;
        ic->i++;
    }else if (ic->shcp == HIGH && shcp == LOW){
        ic->shcp = LOW;
    }
}

So I think the function deserves more documentation (especially why the non state change if ic->shcp == shcp).

Can the next function just not be simplified (I may be missing something in which case a comment):

void ic_set_stcp(struct ic_74hc595 *ic, bit stcp)
{
    if (ic->stcp != stcp) {
        ic->stcp = stcp;
    }
}

Is this not the same as:

void ic_set_stcp(struct ic_74hc595 *ic, bit stcp)
{
    ic->stcp = stcp;
}

OK Reading the function table (page 3) in the linked document:
FUNCTION TABLE See note 1.

I see a set of input and a set of outputs. I also see that the outputs depend on a change in the state of the inputs. As a result I would expect my the functions to mirror this table (maybe this is a bit low level?).

Thus I would have expected an interface like this:

  typedef enum { Low, NoCharge, High, HighImpedence} PinState; 
  typedef enum { Q0, Q1, Q2, Q3, Q4, Q5, Q6, Q7, Q7Dash} OutputPinID;
  struct ic_74hc595   { /* Internal definition */ };

  /*
   * The return value of ic_74hc595_change_inputs() returns an encoded form of the
   * output pins. You can use ic_74hc595_decodeOutputValue() on this returned value
   * to decode individual pins.
   */
  void ic_74hc595_power_up(ic_74hc595* ic);
  int  ic_74hc595_change_inputs(ic_74hc595* ic,
                           PinState SH_CP, PinState ST_CP, 
                           PinState OE,    PinState MR, PinState DS);
  PinState ic_74hc595_decodeOutputValue(int QSlots, OutputPinID pin);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.