Adds padded waves to the API #329

Jul3k · 2020-03-06T10:47:20Z

Task list for releasing this feature:

guymcswain · 2020-03-07T18:37:51Z

@Jul3k , I have other non-GitHub priorities that have come up. I plan to review this in more detail on Tuesday, 3/10.

guymcswain · 2020-03-10T14:57:38Z

Added Task list for releasing this feature. See top comment.

Jul3k · 2020-03-11T15:11:06Z

I would like to add:

Investigate issue when setting percent close to 50% (was size>2 issue before)
Decide if percentage should be specified out of a million as done for PWM dutycycle.

guymcswain · 2020-03-11T15:48:27Z

Moved release task list to the top so we can all make edits. Feel free to self assign tasks you want to contribute to.

guymcswain · 2020-03-11T18:45:29Z

47% appears to be the maximum pad size from my testing. I'll continue to investigate but I would say specifying PPM make little sense with a 3% uncertainty.

guymcswain · 2020-03-12T04:47:36Z

Investigate issue when setting percent close to 50% (was size>2 issue before) @guymcswain

Fails with "No more CBs for waveform" when creating two waves of 48 percent each or 96% of total resource. On further inspection, it appears that 1180 CBs are unavailable for wave consumption or 4.7%.

Total CBs: NUM_WAVE_CBS = 25016
Unavailable CBs: PI_WAVE_COUNT_PAGES*CBS_PER_OPAGE = 1180

Update: Adjusting total CBs to total available CBs (25016-1180) now allows two waves of 50% each.

- gpioWaveCreatePad takes three arguments: %CB, %BOOL, %TOOL - gpioWaveCreatePad checks range of arguments - gpioWaveCreatePad checks dimension of wave fits inside padding - wave2Cbs takes three arguments: numCB, numBOOL, numTOOL - socket command PI_CMD_WVCAP is variadic

guymcswain · 2020-03-12T15:53:18Z

@Jul3k , I pushed changes to the 'wavesize' branch on this repo. I tried to push onto your fork but got error messages. Hopefully you can sync these changes in your fork to update the PR. Let me know.

Wavesize

Jul3k · 2020-03-12T16:21:20Z

@guymcswain I was able to merge with joan2937/wavesize. The PR should already be updated, right? I compiled and tested and all seems good.

guymcswain · 2020-03-13T17:11:39Z

Decide if percentage should be specified out of a million as done for PWM dutycycle.

@Jul3k , Can you present a case that supports making padded wave consistent with PWM?

Jul3k · 2020-03-13T17:34:49Z

If one would like to have 3 buffers of the same size, each buffer would be assigned 33 percent. This would leave 1 percent unassigned. Specifing this out of a million, would only waste one part of a million. As padded waves are likely not filled to the max, this is maybe more an aesthetic issue, but a decission should be made.

guymcswain · 2020-03-13T21:25:17Z

238 control blocks or about 119 pulses under that scenario. I see your point.

Maybe per thousand makes more sense in this case.

guymcswain · 2020-03-19T02:58:15Z

The maximum size wave that can be added using gpioWaveAdd* is 5461 pulses due to the limitation of 65536 bytes for extended parameter buffer size. 50% of the theoretical max wave pulses is 6000. Should we increase the parameter buffer size to accommodate 6000 pulses? More?

guymcswain · 2020-03-19T05:49:35Z

Unfortunately, increasing CMD_MAX_EXTENSION from 65536 to 144000 causes the daemon to crash with:

sigHandler: Unhandled signal 11, terminating

Jul3k · 2020-03-19T08:43:28Z

@guymcswain are you talking about the maximum of around 5000 pulses that can be added by a single gpioWaveAdd call? I experienced a similar issue and I believe the error message was something like connection reset by peer from the python interface.

guymcswain · 2020-03-19T13:52:01Z

Yes, exactly. But I think the logical workaround is to break the pulse train into smaller chunks with a delay at the beginning to align the chunks..

Jul3k · 2020-03-19T14:08:06Z

Yes, I agree if this limitation is useful in terms of an appropriate resource usage or performance wise. Maybe the error message could be more verbose? And also in my case it crashed the pigpio python client which should not happen. As It is not directly related to the padded waves, should we open a new issue on that?

guymcswain · 2020-03-19T14:21:16Z

And also in my case it crashed the pigpio python client which should not happen.

Can a handler be set up in Python to prevent the crash? If not, I would agree it is an issue. If a simple patch is needed then let's handle it here in this PR.

Jul3k · 2020-03-19T14:41:50Z

I believe this should be possible on the python side and maybe the handler can also split a longer pulse table into smaller chunks with a delay at the beginning as you said. This would completely lift that limitation from the user perspective. Should I look into this and do you see any other areas where this might pop up?

guymcswain · 2020-03-19T15:48:47Z

also split a longer pulse table into smaller chunks with a delay at the beginning

Interesting. I was thinking this would just be left to the application. However the Python client is by far the highest usage (>90% I would guess). I could build this into the nodeJs pigpio-client as well. That leaves pigs, pigpiod C/IF and some Java clients without support but probable is ok.

As far as catching the exception, can this not be done easily in the Python application script? In nodeJs for example, I would just add an 'uncaught exception' handler. I'm not fluent in Python.

guymcswain · 2020-03-19T16:04:05Z

Arguing with myself, 😄 , the Python client should range check the P3 argument so the daemon doesn't disconnect. I now think this is the patch that is needed.

I think anything more should be considered an 'enhancement' and be separate from this PR.

guymcswain · 2020-03-19T16:16:58Z

@Jul3k , if you could implement the Python module patch discussed above that would be great. I am in the process of making a rather significant commit that will reduce the number of resources required when generating "differential waves". I'll be pushing this commit later today. Cheers.

joan2937 · 2020-03-19T17:25:02Z

It may be better to trap too large messages in the daemon rather than the client. There should be a central receive point for pipe and socket messages where the length can be checked and an error returned straightaway if too large (I thought I had added such a check, faulty memory syndrome).

guymcswain · 2020-03-19T18:00:19Z

pthSocketThreadHandler is the central point you refer to. Here is the check on P3 argument within the while(1) loop:

pigpio/pigpio.c

Lines 6980 to 7009 in bab05ce

    
           if (p[3]) 
        
           { 
        
              if (p[3] < sizeof(buf)) 
        
              { 
        
                 /* read extension into buf */ 
        
                 if (recv(sock, buf, p[3], MSG_WAITALL) != p[3]) 
        
                 { 
        
                    /* Serious error.  No point continuing. */ 
        
                    DBG(DBG_ALWAYS, 
        
                       "recv failed for %"PRIdPTR" bytes, sock=%d", p[3], sock); 
        
                    closeOrphanedNotifications(-1, sock); 
        
                    close(sock); 
        
                    return 0; 
        
                 } 
        
              } 
        
              else 
        
              { 
        
                 /* Serious error.  No point continuing. */ 
        
                 DBG(DBG_ALWAYS, "ext too large %"PRIdPTR"(%zd), sock=%d", 
        
                    p[3], sizeof(buf), sock); 
        
                 closeOrphanedNotifications(-1, sock); 
        
                 close(sock); 
        
                 return 0; 
        
              }

So are you suggesting the else branch be changed to return an error message and not disconnect? Seems reasonable but I'm not sure how.

guymcswain · 2020-03-19T18:39:52Z

It may be non trivial for the daemon to handle what are essentially bad packets. By disconnecting, it forces compliance on the client.

joan2937 · 2020-03-19T18:45:11Z

Let me have a look and try to remember why i did what I did.

Jul3k · 2020-03-19T20:22:25Z

I had a look if this could be implemented on the client side. Passing a longer list sliced into chunks would not be a big deal and could be done similar like so:

for i in range(0, N, chunk_size):
    pulse_chunk = pulses[i:i+chunk_size]

I see however one Problem. If a, in terms of microseconds, long wave form was initially added and should be merged with a waveform bigger than 5000 pulses, sequentially building up the waveform by adding an initial delay with wave_get_micros does not work, because the duration is determined by the long wave which was added first. Of corse this could be done by the user by keeping track of the total delay he is building up ...

The number of pulses that fits in the wave is limited by the CBs and OOLs right? So could this buffer just be extended to the size that max CBs and OOLs can be fully used? Any larger number of pulses does not make sense anyway and the numbers don't seem to be that far off.

@guymcswain catching the exception is not the problem in Python, the problem is that the socket is closed

guymcswain · 2020-03-19T20:46:18Z

@Jul3k , There is a hard limit on time also:

#define PI_WAVE_MAX_MICROS (30 * 60 * 1000000) /* half an hour */

I tried to increase the buffer size to allow 12000 pulses (max resources of CBs and OOLs) but it broke something. But in any case, the library as written describes how to place waves 'end-to-end' so I don't think a helper function built into the client is needed.

We do need to prevent the socket disconnect from occurring; just need to work out where this is best handled.

Jul3k · 2020-03-19T23:10:11Z

About the socket message size limit I stumbled upon this quote from @joan2937 from this thread

Another hard limit you might reach using the daemon is the maximum socket message size of (I think) 65k bytes. Again I would treat that as a hard hard limit.

I can confirm setting CMD_MAX_EXTENSION (1<<17) fails with “sudo pigpiod -g” terminated by signal SIGSEGV (Address boundary error)

Is this helpful? linux max socket buffer size

guymcswain · 2020-03-20T02:59:30Z

Is this helpful? linux max socket buffer size

Probably not. These buffers are primarily to decouple timing dependencies of the network from the os/application. Ie, you could transfer GBs of data and not fill these buffers.

Jul3k · 2020-03-20T15:27:20Z

By curiosity I changed the Makefile to include debug symbols and hooked pigpiod with CMD_MAX_EXTENSION (1<<17) to gdb. Interestingly the daemon does not crash then and I can send 10k pulses, but for that I need to set a breakpoint, if I let it run without it, it still crashes.

guymcswain · 2020-03-22T17:53:31Z

I have pushed two additional commits to the wavesize branch.

Commit b229b67 corrects issues found when testing with the maximum size wave. There is an additional issue that I have not addressed concerning the number of pulses returned from wave_get_max_pulses(). The number returned is 12000. However, the actual number is 11917. (I'm guessing the discrepancy is for additional resources, both CBs and OOLs, reserved for chaining waves. The complexity is this area of the code was too much for me to cope with for now.)

The other commit, c380873, preserves DMA control blocks when waves are overlapping with opposite polarities.

joan2937 · 2020-03-30T16:01:51Z

The sentinel is fine.

guymcswain · 2020-04-10T17:04:36Z

Decide if percentage should be specified out of a million as done for PWM dutycycle.

I plan to leave the APIs as proposed - using percentages.

guymcswain · 2020-04-29T16:33:25Z

@joan2937 ,
All the tasks items above are completed and I'm preparing to release this in V76. Is there anything I can do to help with the documentation? It's in the source but there is no way to build it from this repo.

joan2937 · 2020-04-29T17:29:03Z

It would be useful if you can check that the documentation builds cleanly. That would show up any formatting errors in the source.

I'm assuming you have a local copy. Go to the DOC directory and run makedoc. It should result in the following.

./makedoc 
*** making man pages ***
pigs
pigpiod
pig2vcd
pigpio.h
pigpiod_if.h
pigpiod_if2.h
*** making web pages ***
cif
download
ex_LDR
ex_ir_remote
ex_motor_shield
ex_rotary_encoder
ex_sonar_ranger
examples
faq
index
misc
pdif
pdif2
pif
pig2vcd
pigpiod
pigs
piscope
python
sif
sitemap

The man pages can then be copied to the main directory.

cp MAN/*.? ..

guymcswain · 2020-04-29T19:42:35Z

Thank you for pointing this out. For some reason my local clone is missing the DOC directory.

joan2937 · 2020-04-29T20:07:11Z

I only added it recently. It should make it easier for others to sensibly improve the documentation.

guymcswain · 2020-04-30T04:55:08Z

The wavesize branch (this PR) has been merged into the develop branch. The docs have been generated, reviewed and corrected from my side. @joan2937 , please take a look at the documentation and let me know if you want to make any changes.

joan2937 · 2020-04-30T11:22:27Z

You can make the following changes to remove the documentation warnings.

pigpio.h
[after *param::]

pctBOOL:: 0-100
percent On-Off-Level (OOL) buffer to consume for wave output.

pctCB:: 0-100
the percent of all DMA control blocks to consume.

pctTOOL:: 0-100
the percent of OOL buffer to consume for wave input (flags).

pigpiod_if2.h
[in wave_create_and_pad]

. .
     pi: >=0 (as returned by [*pigpio_start*]).
percent: 0-100, size of waveform as percentage of maximum available.
. .

[after param::]

percent:: 0-100
The size of waveform as percentage of maximum available.

pigpio.py
[in wave_create_and_pad]

      . .
           pi: >=0 (as returned by [*pigpio_start*]).
      percent: 0-100, size of waveform as percentage of maximum available.
      . .

[after params: 32 bit number]

   percent:: 0-100
   The size of waveform as percentage of maximum available.

guymcswain · 2020-04-30T18:39:23Z

Merged into master as version 76.

@Jul3k , thank you for your contributions!

guymcswain and others added 8 commits March 3, 2020 23:13

Wave create using fixed size CB, OOL

f4a4b97

Skip wave padding when size = 0

58c5ad7

Added .so.* to gitignore

71fe78a

Reverte added site paramezer in wave_create

773378f

Reverted size paramter fore wavecreate in x_pigpio.c

b9b8eff

Added WaveCreatePad to API

4ab125c

Added wave_create_and_pad to pigpio.py

a9f5dc1

Added WaveCreatePad to pigpio.c

c660e3f

Jul3k changed the title ~~Inclueded padded waves in the API~~ Adds padded waves tp the API Mar 6, 2020

Jul3k changed the title ~~Adds padded waves tp the API~~ Adds padded waves to the API Mar 6, 2020

guymcswain added 2 commits March 12, 2020 09:44

Fix error messages on gpioWaveCreatePad

7f05ff4

Merge pull request #1 from joan2937/wavesize

0085f42

Wavesize

guymcswain mentioned this pull request Apr 30, 2020

Release v76 #345

Merged

guymcswain closed this Apr 30, 2020

guymcswain mentioned this pull request Jan 26, 2023

Expose serial number #558

Open

Adds padded waves to the API #329

Adds padded waves to the API #329

Conversation

Jul3k commented Mar 6, 2020 • edited by guymcswain Loading

guymcswain commented Mar 7, 2020

guymcswain commented Mar 10, 2020 • edited Loading

Jul3k commented Mar 11, 2020

guymcswain commented Mar 11, 2020

guymcswain commented Mar 11, 2020

guymcswain commented Mar 12, 2020 • edited Loading

guymcswain commented Mar 12, 2020

Jul3k commented Mar 12, 2020

guymcswain commented Mar 13, 2020 • edited Loading

Jul3k commented Mar 13, 2020

guymcswain commented Mar 13, 2020 • edited Loading

guymcswain commented Mar 19, 2020 • edited Loading

guymcswain commented Mar 19, 2020

Jul3k commented Mar 19, 2020

guymcswain commented Mar 19, 2020

Jul3k commented Mar 19, 2020

guymcswain commented Mar 19, 2020

Jul3k commented Mar 19, 2020

guymcswain commented Mar 19, 2020

guymcswain commented Mar 19, 2020 • edited Loading

guymcswain commented Mar 19, 2020 • edited Loading

joan2937 commented Mar 19, 2020

guymcswain commented Mar 19, 2020

guymcswain commented Mar 19, 2020 • edited Loading

joan2937 commented Mar 19, 2020

Jul3k commented Mar 19, 2020 • edited Loading

guymcswain commented Mar 19, 2020 • edited Loading

Jul3k commented Mar 19, 2020 • edited Loading

guymcswain commented Mar 20, 2020

Jul3k commented Mar 20, 2020

guymcswain commented Mar 22, 2020

joan2937 commented Mar 30, 2020

guymcswain commented Apr 10, 2020

guymcswain commented Apr 29, 2020

joan2937 commented Apr 29, 2020

guymcswain commented Apr 29, 2020

joan2937 commented Apr 29, 2020

guymcswain commented Apr 30, 2020

joan2937 commented Apr 30, 2020

guymcswain commented Apr 30, 2020

Jul3k commented Mar 6, 2020 •

edited by guymcswain

Loading

guymcswain commented Mar 10, 2020 •

edited

Loading

guymcswain commented Mar 12, 2020 •

edited

Loading

guymcswain commented Mar 13, 2020 •

edited

Loading

guymcswain commented Mar 13, 2020 •

edited

Loading

guymcswain commented Mar 19, 2020 •

edited

Loading

guymcswain commented Mar 19, 2020 •

edited

Loading

guymcswain commented Mar 19, 2020 •

edited

Loading

guymcswain commented Mar 19, 2020 •

edited

Loading

Jul3k commented Mar 19, 2020 •

edited

Loading

guymcswain commented Mar 19, 2020 •

edited

Loading

Jul3k commented Mar 19, 2020 •

edited

Loading