Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acoustic event detector #46

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

longle2718
Copy link

Hi guys,
This is our universal acoustic event detector. Correct operation still depends on the ADC driver though.
Currently, all source files are copied-and-pasted from another debugging repos (https://bitbucket.org/longle1/gcwa) and laid out flat to build with the default Makefile. I could use some help with the Makefile to reorganize the code and avoid copy-and-paste everytime there's an update in the original repo. :)
Thanks,
Long

APP_DIR := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))

# files needed for this code
C_SRCS := $(wildcard *.c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the Makefile variable you want to adjust if you want to have a non-flat hierarchy. I believe something like this would include all C files in all subdirectories work:

C_SRCS := $(wildcard *.c)
C_SRCS += $(wildcard **/*.c)

You could also be more explicit if you only want to include certain subdirectories or files.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Amit,
Thank you so much for your help! I tried

C_SRCS   := $(wildcard *.c)
C_SRCS   := $(wildcard **/log2fix.c)

but still get

DEP        log2fix/log2fix.c
<built-in>: fatal error: opening dependency file /mnt/c/Users/Long/Projects/signpost/software/apps/audio_module/event_detector/build/cortex-m4/log2fix/log2fix.d: No such file or directory
compilation terminated.

Namely, the Makefile still looks under the build/ but didn't copy the source files into it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh... yes you are right... the problem is that there is no lib2fix subdirectory in the build/cortex-m4 directory and our Makefile rule assumes there is (and GCC won't just create one). If you create one after the build fails the first time, it will be able to move past it, but that's not solution.

@ppannuto please halp! You are the resident dependency file expert!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick follow-up on this motivated by today's phone call. This should be largely fixed by the updates to the Tock build system. I updated signpost a few days ago in the tock-master branch, but we're holding off on merging that until after the paper deadline. I'll follow up again on April 11 :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Pat,
Thanks for the follow-up. And no worries at all, I understand the burden of a paper deadline. We'll get this working soon.
In the meantime, godspeed to your paper!

@@ -0,0 +1,14 @@
# Fixed-point ridge tracker for acoustic event detection in C
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this README is just copied from the original repo, and maybe with proper modifications to the Makefile, that's just a submodule or something. However, I think it would be good to have a signpost-port-specific README that

  1. explains a bit more about what the app actually does (e.g. "Detects acoustic events", what "acoustic events" are, etc),

  2. and gives instructions relevant to Signpost (e.g. you can't run main with arguments on Signposts).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your guess is correct! I've updated the README according to your suggestion. Thank you.

Use test.ipynb to visualize the resulting \*.mat files

## Prerequisites
* Fixed-point FFT: git clone https://github.com/longle2718/kiss_fft
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these dependecies are vendored (which I believe you've done) I don't think they need to be listed as prerequisites.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. However, once we have a hierarchical directory structure, we won't be copying the sources and just leave them as prerequisites. Thanks!

a->used = 0;

a->size = a->maxSize;
printf("Init array size = %zu\n",a->size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a debug message? Perhaps better for library code to do this in a DEBUG_PRINTF macro that can be turned off at compiled time? (As opposed to printfs from the actual application code, like in main.c)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for telling me about DEBUG_PRINTF! This will be fixed in the next pull request.

return 0;
}

static int freqAnalyze(kiss_fft_scalar *frame, kiss_fft_scalar *spec){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, these static helper methods are returning positive integers on error, which is pretty unintuitive for C. I think it would be better to return negative numbers (common for errors) and check if(helper_function() < 0) in main.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you very much for telling me about the correct style in C. I'll fix this accordingly.

}

static int printMat(char *filename, kiss_fft_scalar *in, size_t N){
FILE *fp;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... There is no support currently for storing files. In principal, we could implement a filesystem that ships over to the SD card on the main board, but currently I believe it will always fail on fopen.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I suspected, too. I plan to replace these printMat with the appropriate Signpost's memory API to log results. We'll definitely need to ship events out of the Signpost for more processing.

@alevy
Copy link
Contributor

alevy commented Mar 10, 2017

This is cool! I guess the goal is to eventually either ship events out over the radio or use this as a library for a larger application that uses the events as a signal to start doing other processing?

@longle2718
Copy link
Author

Hi Amit,

Thank you so much for your time and patience with my first pull request! I really appreciated your guidance.

Yes, my current goal is to ship events out of the Signpost for more processing. In my experience on the Android platform, the right way to do this is to write detected events down to a local storage queue, and depends on a "Network/Communication Manager" to clear the queue/ship events off the board depending on the network state/energy availability. Thoughts?

The event detector you see is indeed a subroutine/library in a larger processing chain. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants