GIDForums  

Go Back   GIDForums > Computer Programming Forums > C++ Forum
User Name
Password
Register FAQ Members List Calendar Search Today's Posts Mark Forums Read

 
 
Thread Tools Search this Thread Rate Thread
  #1  
Old 08-Jan-2007, 08:12
clarjon1 clarjon1 is offline
New Member
 
Join Date: Jan 2007
Posts: 2
clarjon1 is on a distinguished road

Could someone please look this over?


Hello all.
I'm teaching myself programming in C++, for a school project.
I know this isn't a "Help me with homework" place, so please hear me out.
First off, I don't want to be like the rest of the class, and go with QBasic. I've done most of the code, but I'm not too sure if everything is where it should go.
I'm trying to control a coffe maker via parallel port, turning it on and off.
Could you please check my code and see if the commented out outb and ioperm commands are in the right spot?
And please let me know if I will need any additional headers for what I want to do.
Thanks!

Jon
Attached Files
File Type: txt main.cpp.txt (4.6 KB, 20 views)
  #2  
Old 08-Jan-2007, 10:34
WaltP's Avatar
WaltP WaltP is offline
Outstanding Member
 
Join Date: Feb 2004
Location: Midwest US
Posts: 3,243
WaltP is a name known to allWaltP is a name known to allWaltP is a name known to allWaltP is a name known to allWaltP is a name known to allWaltP is a name known to all

Re: Could someone please look this over?


Quote:
Originally Posted by clarjon1
Hello all.
Hi

Quote:
Originally Posted by clarjon1
I've done most of the code, but I'm not too sure if everything is where it should go.
I'm trying to control a coffe maker via parallel port, turning it on and off.
Could you please check my code and see if the commented out outb and ioperm commands are in the right spot?
Neither are we. It's your program. Does the placement make sense for what you are trying to do?

The one thing I do see is you'll never actually see the coffee maker on. The code will run so fast it'll just blip quickly. Try putting a wait before you turn it off, like another cin or something.

The big question is -- does it compile? If so, great! If not, fix the errors and get it running. That's the best test of all.

And please post the code in the post, not as an attachment. Most people won't bother with reading an attachment.
__________________

Age is unimportant -- except in cheese
  #3  
Old 09-Jan-2007, 12:11
davis
 
Posts: n/a

Re: Could someone please look this over?


Quote:
Originally Posted by clarjon1
Hello all.
I'm teaching myself programming in C++, for a school project.
I know this isn't a "Help me with homework" place, so please hear me out.
First off, I don't want to be like the rest of the class, and go with QBasic. I've done most of the code, but I'm not too sure if everything is where it should go.
I'm trying to control a coffe maker via parallel port, turning it on and off.
Could you please check my code and see if the commented out outb and ioperm commands are in the right spot?
And please let me know if I will need any additional headers for what I want to do.
Thanks!

Jon

Jon,

Your code is very typical of someone without much experience in writing/designing good C++ classes. I don't mean to be overly critical or harsh, but you will want to separate out the Parallel Port control "stuff" from the actual implementation of the CoffeeMakerController class so that the CoffeeMakerController uses the "ParallelPortIO" class' features. What you've got now is an integration of "all things related to the project" in a single class named "Coffee." By convention and by preference, we need to take the activity of naming our classes very seriously. A class named Coffee should represent what Coffee is. Since the compiler doesn't know a good name from a bad name, it will happily let you name a class "Rock" with operations named "calculate_rubix_cube_moves" whether there is any real relationship between what a Rock IS and what a Rock DOES and/or whether or not any of that has anything to do with Rubix cubes. By convention, as C++ programmers who create classes, we MUST ensure that the class IS what we say it is in that whatever we'd expect a "Coffee" (or a Rock) to be or do, our design must reflect that basic and general understanding...or else it leads to unending confusion.

Here is a very simple ParallelPortIO class that I wrote for you to integrate into your code. I did not exhaustively test it in any way other than what you see being done in the included main.cpp. However, it is drawn from my previous experiences using parallel ports to control external devices and to read/write data to such devices.

I include this code so that you can see how I would separate out the functionality of what is being done by the ParallelPortIO class so that you can write a CoffeeMakerController class that USES the parallel port IO functionality without "integrating" that code inside something which is NOT related to parallel port IO in any way...except as a function of being a "user" of that functionality.

ParallelPortIO.h

CPP / C++ / C Code:
#ifndef _ParallelPortIO_h_
#define _ParallelPortIO_h_ 1

#include <stdexcept>

#include <unistd.h>
#include <sys/io.h>


class ParallelPortIO
{
public:

    // by design, this class only supports LPT1 & LPT2
    const static unsigned int  LPT1              = 0x378;
    const static unsigned int  LPT2              = 0x278;

    const static unsigned char LPT_PD_ALL_OFF    = 0x00;
    const static unsigned char LPT_PD0           = 0x01;
    const static unsigned char LPT_PD1           = 0x02;
    const static unsigned char LPT_PD2           = 0x04;
    const static unsigned char LPT_PD3           = 0x08;
    const static unsigned char LPT_PD4           = 0x10;
    const static unsigned char LPT_PD5           = 0x20;
    const static unsigned char LPT_PD6           = 0x40;
    const static unsigned char LPT_PD7           = 0x80;
    const static unsigned char LPT_PD_ALL_ON     = 0xFF;

    const static unsigned char LPT_PORTS_INPUT   = 0x50;
    const static unsigned char LPT_PORTS_OUTPUT  = 0x00;

    ParallelPortIO() { init( LPT1 ); }
    ParallelPortIO( const int port ) { init( port ); }

    void setToInputMode()  { outb( LPT_PORTS_INPUT, m_control ); }
    void setToOutputMode() { outb( LPT_PORTS_OUTPUT, m_control ); }
    void setPin( const unsigned char pin, const bool high=false ) { outb( (high ? pin : ~pin), m_data ); }
    unsigned char readByte() { unsigned char byte = inb( m_port ); return byte; }
    void writeByte( const unsigned char byte ) { outb( byte, m_data ); }
    void setBaseIOAddress( const unsigned int base ) { init( base ); }
    const unsigned int getBaseIOAddress() const { return m_port; }

protected:
private:
    int m_port;
    int m_data;
    int m_status;
    int m_control;
    void init( const unsigned int port )
    {
        if( port == LPT1 || port == LPT2 )
        {
            m_port = port;
        }
        else
        {
            m_port = LPT1;
        }
        m_data      = m_port;
        m_status    = m_data +1;
        m_control   = m_data +2;
        if( ioperm( m_port, 3, 1 ) )
        {
            throw new std::domain_error( "root access required" );
        }
        else
        {
            setToOutputMode();
            writeByte( LPT_PD_ALL_OFF ); // start with all pins low
        }
    }
};


#endif // ! _ParallelPortIO_h_

main.cpp

CPP / C++ / C Code:
#include <iostream>

using namespace std;

#ifndef _ParallelPortIO_h_
#include <ParallelPortIO.h>
#endif


int main()
{
    ParallelPortIO ppio;
    ppio.writeByte( ParallelPortIO::LPT_PD_ALL_ON );
    cout << "LPT1 port pins should all be HIGH..." << endl;

    char wait;
    cin >> wait;

    ppio.writeByte( ParallelPortIO::LPT_PD_ALL_OFF );
    cout << "LPT1 port pins should all be LOW..." << endl;

    cin >> wait;

    return 0;
}

...here you can see how main uses a ParallelPortIO instance called ppio. You will likely want to do something similar with your CoffeeMakerController class. I can make recommendations for additional abstractions that are relevant and useful to understand, however, they tend toward being more true to the idea of the Open/Closed Principal (which states that the class is open for extension, closed for changes) and, as such, tend toward making the design flexible enough to support IO subsystems that would be inclusive of other types of IO, such as Ethernet, IrDA, etc., and not simple exclusive to PPIO. Most aspiring C++ programmers tend to think of this "work" as severe overkill considering that all they want to do is toggle a few pins on a parallel port to do some basic work...however, it is "proper" C++ and teaching others this fact is always a constant struggle.

You'll note that the ioperms requirement is only found in the init. A "proper" implementation of my PPIO class MUST try/catch for the exception thrown by init, but I was lazy and beginning to get close to my 10-minutes maximum response time. I tell you now so that you can modify it and handle it, because a ctor is not allowed to fail. In case of non-root access, the exception will be thrown and cause the ctor to abort, which is not "good."

As a point of order, when you separate the roles and responsibilities of a project into an appropriate set of classes, you also gain some organizational benefit because your code is now neatly organized into logical "units" of composite functionality. You can individually write tests for each of these "units" that stand-alone and prove that portion of the entire project is working properly. Also, when you separate things by roles and responsibilities, you gain the promise of reusability in that the next time that you need to use a parallel port, you can simply #include your parallel port I/O class and begin using it by its public interfaces. You can see that if you embed your inb/outb calls inside of your Coffee class, that you can't really ever use them for the next time that you need to read/write to the parallel port.

HTH...


:davis:
  #4  
Old 09-Jan-2007, 12:59
davis
 
Posts: n/a

Re: Could someone please look this over?


Quote:
Originally Posted by WaltP
Neither are we.

WaltP...you may want to confine your answers to answering just for yourself. Some of us know whether his "planned" uses of ioperm and outb are "in the right places" or not. The basic reply to his question isn't whether it compiles successfully or not, since that isn't what he was asking.

Jon, the answer is that you only need to call ioperm once to see if you've got access to the port. If so, then you need to call outb anytime that you want to write a byte on the port. My previous reply should help guide you toward a set of interfaces that will make it easier for you to use this functionality.

Note that in some arguments that could be made, there is no difference in adding my PPIO class' writeByte operation versus the outb function to your own implementation code. Let me try to explain:

Ideally, one would create a generic CoffeeMakerIO class that would be an abstract base class for all the possible IO types that might be assoicated with CoffeeMaker(s). Then, you would see an inheritance hierarchy that would have CoffeeMakerIO as the base for several IO classes including ParallelPort and EthernetPort, IrDAPort...etc. As you may guess, CoffeeMakerIO would have the top-level functional operations such as:

setBurner( bool on=false );
setTimer( time_t t );

...etc.

Then, each IMPL class (e.g. CoffeeMakerPPIO, CoffeeMakerEthIO, etc.) would then aggregate (aka composition) the class(es) needed for the fundamental IO (an example is my PPIO class) AND support the operations required by the abstract base class (ABC), CoffeeMakerIO. That is:

CoffeeMakerPPIO would have a setBurner and setTimer operation (and others required by the ABC) AND it would interact with the "low-level" IO class (such as PPIO/ETHIO) in whatever way was necessary to faciliate the intended operational characteristics. So, if PD0 high turns on the burner:

CPP / C++ / C Code:
void CoffeeMakerPPIO::setBurner( bool on )
{
    m_ppio->setToOutputMode();
    m_ppio->setPin( ParallelPortIO::LPT_PD0, on );
}

...we can see how this interface "works" for other IO subsystems, since whatever work is required to turn the burner on by another IO subsystem would simply be written inside of the block of code for that CoffeeMaker_XXX_IO::setBurner implementation. In the above code snippet, m_ppio would be an instance of the ParallelPortIO class (with any necessary fixes) that I wrote in my previous reply.

What this kind of "design" does for us is that it allows us to come along later and implement EtherIO if we so decide that is of use. It also allows us to implement any other IO subsystem that we perhaps haven't considered. What this does for us is gives us flexibility in our code to support changes that we certainly expect will happen over time, but that we can not definitely say are necessary at this time. Some who argue against this kind of coding use the argument that it is too much work for something that will never need an Ethernet-based IO. Sure, if it never needs to be changed, it is "added work." However, as I hope that I've shown, the next time that a low-level PPIO interface is needed, we can still reuse a portion of the work AND if we follow these kinds of design choices, particularly where we expect changes to more likely occurr, we will know how to use them and support the changes without barfing up a lot of dead code.

The above code that shows the setBurner interface also allows other "users" inside your own code that need to turn the burner on or off to use a consistent interface rather than each of them having their own outb function calls. This gives you the ability to change the code in one place in such situations where the "prototype board" differ from the "production board" which does occurr in real life. For example, if a hardware engineer decides to change the PD0 line to PD1, rather than replacing perhaps a dozen calls to outb with an argument of PD0 to an argument of PD1, we simply change the one call in setBurner. More subtle situations where this is of value is when we later decide that we don't only want to just blindly raise and lower a pin state to (hopefully) do something meaningful, rather, we want to interrogate the device to see if the burner state is changeable and/or that its "limits" are within some range or whatever, we can hide that functionality inside of the interface so that we can perform any necessary logic. An example might be in the case of a PWM where we need to change the pluse duration in the case where the burner is not a simple on/off, but perhaps controlled by configuring a PWM circuit in our controller. Maybe we need to send a data packet? Maybe we don't need to change it if the burner is already at maximum temperature? Maybe if it is in an "over temp" condition, we want to reduce it slightly? Everywhere that someone wants the burner on, we point them to the "BurnerControllerManager" interface through the setBurner interface in the IO abstraction layer. Maybe this is too much information for some, but hopefully my point won't be completely lost on those interested in learning WHY we C++ and not just C.


:davis:
  #5  
Old 10-Jan-2007, 07:05
clarjon1 clarjon1 is offline
New Member
 
Join Date: Jan 2007
Posts: 2
clarjon1 is on a distinguished road

Re: Could someone please look this over?


Thanks for your help, guys.
I realize I wasn't very clear.

This is my first real program, which I have been just putting stuff together, and moving around, as I go along and understand the language better. First off, the reason why it would just blip the pins on and off is because I haven't gotten the (what I see as being the easiest to implement) while loop for input from the user as to whether the pin is to be on or off. It does compile right now, I've been very careful of that.
Thanks for your detailed responses, I really appreciate it. And don't worry, I don't mind a bit of bashing for something that i did very wrong. Helps me know what to do. The reason why it's in C++ is because I'm hoping to eventually figure out how to put the code into Qt.

I'll try separating my code in the future. I'll let you know how things turn out!

PS: You don't need to stop with the suggestions, or tips for someone teaching themselves the language!
 
 

Recent GIDBlogMeeting the local Iraqis by crystalattice

Thread Tools Search this Thread
Search this Thread:

Advanced Search
Rate This Thread
Rate This Thread:

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Forum Jump

Network Sites: GIDNetwork · GIDWebHosts · GIDSearch · Learning Journal by J de Silva, The

All times are GMT -6. The time now is 14:26.


vBulletin, Copyright © 2000 - 2008, Jelsoft Enterprises Ltd.