I like your comments throughout.  Gives me a feel of the why or what you were 
trying to accomplish rather than something like 
a = 5; // assign 5 into variable a. 

Being nit picky I'd have done the following with the opposite logic.

    // --- beg --- create & export hal_pins
    for (n = 0; n < npins; n++) {
      if( pinarray[n].gpionum ){ 
        // NADA  fall into ensuing code
      }else{   /* this pin is not used so skip to top of loop       */
        continue;
      }

Prevents the imperceptible pause while reading the text causing one to stop, 
move up a few lines lines and go "Ah.  The pin was 0 so not used.  That's what 
he's doing"

    // --- beg --- create & export hal_pins
    for (n = 0; n < npins; n++) {
      if( pinarray[n].gpionum == 0 ){ 
        continue;   // this pin is not used so skip to top of loop      
      }

Are you going to be able to link SPI or I2C to this or maybe even socketCAN via 
SPI based MCP2515 devices?  Is that even possible in the HAL?

Other than that from a documentation perspective I'd like to see a bit more why 
something is done.  For example:  
int get_initial(char * cp) {

Searched the entire file.  No other function calls this.  So it must be 
required for a component?  In which .h file is it exported.  Yes. I can find 
those files and look in each one to find out if it's even declared as global.   
A header in the function that states where it's exported and why or who would 
call it and when.  I'm guessing cp means character pointer?

My comments are based on not knowing how to write a component and not knowing 
what's needed.

Great job otherwise.
John


> -----Original Message-----
> From: Thomas J Powderly [mailto:tjt...@gmail.com]
> Sent: February-16-21 7:49 AM
> To: emc-develop...@lists.sourceforge.net; emc-users@lists.sourceforge.net
> Subject: [Emc-users] hal_pi_gpio man page
> 
> The hal_pi_gpio component seems to be beta stage
> meaning I don't see anything wrong.
> When I programmed full-time this was when I had to hand it over to
> others to peer.
> 
> Where do I look for howto write a man page?
> 
> About the comp -----------------------------------------
> Written in old-school c
> For rpi's only
> ( so far anyway, the scheme is flexible for similar, opi bpi maybe bbb
> bbw bbg )
> 
> It's real-time ( well it starts with� loadrt under userspace linuxcnc
> tested on rpi33 )
> It only needs 1 thread and has re-settable pins
> ( and has a setp'able param reset_time ,
>  � set in integer of nS units (up to 1/4 thread period))
> It has inputs and in-nots
> It has outputs and out-inverts and out-resets
> It uses a man readable config format ( a teeny language to describe each
> pin , see end of msg)
> It reads switches and blinks leds
> ( an old boss assured me more blinky lights was a good thing )
> 
> I can send a copy to anyone wanting to try, hell its text, see attachment.
> 
> How to build in a rip installation:-------------------------
> I compile it by backing up the hal_pi_gpio.c in my rip directory
>  � ~/linuxcnc-dev/src/hal/drivers/hal_pi_gpio.c� (move to another dir,
> don't just rename it)
> then copying my file into there using the same name as the pi gpio.c
> file that was there before.
> 
> Then touch it (to insure it is changed so make will notice and rebuild it )
>  �type�� touch hal_pi_gpio.c�� in the src/hal.driver dir
> 
> Then���� make modules�� from the ~/linuxcnc/src�� dir
> (this trickery is due to the .c file
>  �needing other .c files in that dir,
>  �and a simple gcc command is beyond my ken )
> I get no errors no notes no warnings, its a clean make here
> 
> Once it makes, you use it with halrun like this
> 
> halrun
> loadrt hal_pi_gpio pi_pins={03,<,+,x}{05,>,1,Y}{07,>,0,}{40,>,1,Y}
> loadrt threads name1=slow period1=150000
> addf hal_pi_gpio.read slow
> add hal_pi_gpio.write slow
> addf hal_pi_gpio.reset slow
> setp hal_pi_gpio.reset_time 30000 (easier to see than my 2000)
> start
> 
> 
> Then use show pin and show param to watch things work
> (and halmeter and halscope too)
> 
> You'll need a scope to catch the reset pins.
> 
> They change inside one thread period so halscope cant see 'em.
> 
> Hook up a bread board and switches and leds ( 3.3V i/o here!!)
> 
> ( as simple as hooking up an output to an input will show you something )
> 
> About the pi_pins config
> line--------------------------------------------------
> It's simple but strict rules
> Each pin is described by a 'quad' ( a 4 param thingy )
> Each quad has the format _like_
> {03,>,0,N}
> This describes the rpi header pin#3
>  ���� and is an output ( the '>' is used for out )
>  ���� and has an initial power on value of 0 ( zero )
>  ���� and does NOT have a reset feature
> {05,>,1,Y}
>  ��� describes header pin 5
>  ��� which is an output '>'
>  ��� and has intial value of 1
>  ��� and has the reset feature ( requires set hal_pi_gpio.reset_time
> 2000 for 2us example)
> {07,<,+,x}
>  ��� describes header pin 7
>  ��� which is an input
>  ��� and has an in-not pin due to '+'
>  ��� and the last param is meaningless, tested lightly with a few single
> chars in that place
> {12,<,-,x}
>  �� describes header pin 12
>  �� which is an input
>  �� and does not get an in-not pin , due to '-'
>  �� and last parm is meaningless
> 
> 
> hokay
> I am sure i didn't write near 1000 lines w/o mistakes
> lemmeno
> tomp




_______________________________________________
Emc-users mailing list
Emc-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/emc-users

Reply via email to