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

support for raspyrfm hardware #431

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

Phunkafizer
Copy link

Add support for the RaspyRFM 434/868 module: https://www.seegel-systeme.de/produkt/raspyrfm-ii/

@CurlyMoo
Copy link
Contributor

Finally, RaspyFM 👍

Can you change the license to MPLv2?

Also, you do have the posibility to make a unit test for it as well on the rewrite branch?

@CurlyMoo
Copy link
Contributor

Also, don't forget to include that manual page.

@Phunkafizer
Copy link
Author

Finally, RaspyFM 👍

Can you change the license to MPLv2?

Also, you do have the posibility to make a unit test for it as well on the rewrite branch?

Can you give me some hints how to do that?

@CurlyMoo
Copy link
Contributor

@Phunkafizer Phunkafizer force-pushed the raspyrfm branch 2 times, most recently from 961e27f to 4292759 Compare January 17, 2019 19:27
@Phunkafizer
Copy link
Author

This is my 433gpio unittest:
https://github.com/pilight/pilight/blob/rewrite/tests/hardware_433gpio_receive.c
How do I run this unittest?

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Jan 17, 2019

Here is the compile command:
https://github.com/pilight/pilight/blob/rewrite/ci/travis-script.sh


Also, i'm fine with a seperate hardware module folder like in the protocols, but currently all hardware modules descriptions are put in the configuration/hardware.rst file. If you want to keep the current setup, make sure to include your file in there (at the end of that file):

433.92Mhz / 868Mhz
------------------

.. include:: ../hardware/_raspyrfm.rst

Also, you documentation file has some issues, this fixes it:

.. |yes| image:: ../images/yes.png
.. |no| image:: ../images/no.png

.. role:: underline
   :class: underline

+------------------+-------------+
| **Feature**      | **Support** |
+------------------+-------------+
| Sending          | |yes|       |
+------------------+-------------+
| Receiving        | |yes|       |
+------------------+-------------+
| Config           | |yes|       |
+------------------+-------------+

.. rubric:: Config

Example configuration for a RaspyRFM single 868 MHz

.. code-block:: json
  :linenos:

  {
    "hardware": {
      "raspyrfm": {
        "spi-channel": 0,
        "frequency": 868350
      }
    }
  }
  
Example configuration for a RaspyRFM single 433 MHz

.. code-block:: json
  :linenos:
	
  {
    "hardware": {
      "raspyrfm": {
        "spi-channel": 0,
        "frequency": 433920
      }
    }
  }


.. note:: **RaspyRFM**

   The RaspyRFM is a radiomodule using the RFM69 from HopeRF.
   https://www.seegel-systeme.de/produkt/raspyrfm-ii/

The image path needs to be changed, when including that file in the hardware.rst.

Also, currently all hardware modules currently support both sending and receiving, and they all need to be configured to be used. So the feature table is not usefull.

@Phunkafizer
Copy link
Author

Also, you do have the posibility to make a unit test for it as well on the rewrite branch?

Why having 2 PRs with the same content? What is the purpose of the "staging" branch, what of the "rewrite" branch?

@CurlyMoo
Copy link
Contributor

I understand it's confusing. You were not the first asking that question, so i will refer to the previous answer.
https://forum.pilight.org/showthread.php?tid=3504

@Phunkafizer
Copy link
Author

https://github.com/pilight/pilight/blob/rewrite/ci/travis-script.sh

Running this on the PI, nothing happens

https://github.com/pilight/pilight/blob/rewrite/tests/hardware_433gpio_receive.c

Sorry I did not really understand how this works. How could I write a unit test for the raspyrfm?

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Jan 18, 2019

Running this on the PI, nothing happens

Don't run the bash script, check the cmake command to see how you compile with unit test enabled.

Once you got it compiled, run sudo LD_PRELOAD=libgpio.so ./pilight-unittest list for all unittest available, to run a specific unittest run sudo LD_PRELOAD=libgpio.so ./pilight-unittest test_hardware_433gpio_receive

You can see the output of this all here on travis:
https://travis-ci.org/pilight/pilight/builds/478986505

Sorry I did not really understand how this works. How could I write a unit test for the raspyrfm?

What i did for the 433gpio library is writing a dummy libgpio.so shared library which is preloaded with LD_PRELOAD. Therefor the functions inside libwiringx.so are overwritten by those in libgpio.so. What libgpio.so does is letting you create unix pipes to mimic gpio.

On this line a file socket is being created https://github.com/pilight/pilight/blob/rewrite/tests/hardware_433gpio_receive.c#L110-L117 called /dev/gpio1.

I use that socket here to mimic activity on /dev/gpio1 as if it where real interrupts: https://github.com/pilight/pilight/blob/rewrite/tests/hardware_433gpio_receive.c#L68-L81

Inside libgpio.so the same thing happens but from the other side. So the function pinMode will now try to connect to that named socket to read as if it where real gpio:
https://github.com/pilight/pilight/blob/rewrite/tests/libgpio.c#L71-L91

The only difference from real gpio is that we cannot read with UV_PRIORITIZED because that only works on real sockets and real gpio. So instead we read from UV_READABLE.

So what i'm doing is just mimicing real GPIO activity as you would normally create with a scope, but now done fully software wise.

@Phunkafizer
Copy link
Author

Thank you for the detailed explaination! I see a bit clearer now. However writing a unittest for the RaspyRFM could be complex as I would have to emulate the behavior of the Chip with all the used registers. I'm afraid I can not contribute that.

@CurlyMoo
Copy link
Contributor

However writing a unittest for the RaspyRFM could be complex as I would have to emulate the behavior of the Chip with all the used registers. I'm afraid I can not contribute that.

The problem is that if we don't get that unittest i can't guarentee the functionality of the RaspyRFM in future versions. As you might have noticed, i'm currently porting all modules to lua. At the moment, i'm rewriting all hardware modules to lua. If i don't have a unittest, the hardware module just won't be supported, because i have no way to test against when porting the current code to lua.

Also, writing i2c unittests isn't that difficult as i2c can be emulated pretty easily. You can see an example here where i wrote one for the BMP180, LM75 and the LM76.
https://github.com/pilight/pilight/blob/rewrite/tests/protocols_i2c.c

But, the great thing about the lua modules is that i don't have give much about third party modules, because issues can be fixed super easy without having to recompiling the pilight core.

@Phunkafizer
Copy link
Author

Running the unittest for the gpio I get:

`sudo LD_PRELOAD=libgpio.so ./pilight-unittest test_hardware_433gpio_receive
[ test_hardware_433gpio_receive ]
F

There was 1 failure:

  1. test_hardware_433gpio_receive: /home/pi/dev/pilight/tests/hardware_433gpio_receive.c:226: assert failed

!!!FAILURES!!!
Runs: 1 Passes: 0 Fails: 1
`

What does this mean?

@CurlyMoo
Copy link
Contributor

That the test failed. But this specific test wasn't implemented well. The new lua unittest doesn't have this issue. The other tests should succeed as well.

@Phunkafizer
Copy link
Author

I tried to implement a skeleton of a unittest for the RaspyRFM but the test does not appear in the list. What did I miss? See

Phunkafizer@1ac80b5

@CurlyMoo
Copy link
Contributor

@Phunkafizer
Copy link
Author

Ok, I updated my branch

Phunkafizer@b0ba17a

Meanwhile my hardwaremodule's init is called but the unittest failes at this point:

if(config_setting_get_string("gpio-platform", 0, &platform) != 0) { logprintf(LOG_ERR, "no gpio-platform configured"); return EXIT_FAILURE; }

Where do I have to set the platform? I could not figure it out in the 433gpio unittest.

@CurlyMoo
Copy link
Contributor

You should configure it first in the config:
https://github.com/pilight/pilight/blob/rewrite/tests/hardware_433gpio_receive.c#L179-L185

@Phunkafizer
Copy link
Author

Meanwhile my IRQ routine in the hardwaremodule is called by the unittest.

Also, writing i2c unittests isn't that difficult as i2c can be emulated pretty easily.

The RaspyRfm is actually using SPI, not I2C. How can I connect "the other side" from software?

@CurlyMoo
Copy link
Contributor

I've build a proof-of-concept that might work:

Place all files in the cmake build folder

libgpio.c

/*
	Copyright (C) 2013 - 2016 CurlyMo

  This Source Code Form is subject to the terms of the Mozilla Public
  License, v. 2.0. If a copy of the MPL was not distributed with this
  file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <string.h>
#include <assert.h>
#include <stdarg.h>
#include <unistd.h>
#include <sys/time.h>
#include <sys/socket.h>
#include <sys/un.h>
#define __USE_GNU
#include <dlfcn.h>

#include "../libs/pilight/core/log.h"
#include "../libs/pilight/core/mem.h"
#include "../libs/libuv/uv.h"

static int state[255] = { 0 };
static uv_os_fd_t fd[255] = { -1 };
static int (* ioctl_callback)(int fd, unsigned long req, void *cmd) = NULL;

#ifdef _WIN32
__declspec(dllexport)
#endif
int wiringXSetup(char *name, void (*func)(int, char *, int, const char *, ...)) {
	if(name != NULL && strcmp(name, "test") == 0) {
		return -999;
	}
	int i = 0;
	for(i=0;i<255;i++) {
		fd[i] = -1;
		state[i] = 0;
	}
	if(name == NULL || strcmp(name, "gpio-stub") == 0) {
		return 0;
	}
	return -1;
}

int wiringXValidGPIO(int gpio) {
	if(gpio == 0 || gpio == 1 || gpio == 2 || gpio == 14 || gpio == 10) {
		return 0;
	}
	return -1;
}

int digitalWrite(int gpio, int mode) {
	return ((send(fd[gpio], "a", 1, 0) == 1) ? 0 : -1);
}

char *wiringXPlatform(void) {
	return "gpio-stub";
}

int digitalRead(int gpio) {
	state[gpio] ^= 1;
	return state[gpio];
}

int wiringXSelectableFd(int gpio) {
	return fd[gpio];
}

int pinMode(int gpio, int mode) {
	struct sockaddr_un address;

	if(fd[gpio] == -1) {
		if((fd[gpio] = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
			logprintf(LOG_ERR, "socket");
			return -1;
		}

		/* start with a clean address structure */
		memset(&address, 0, sizeof(struct sockaddr_un));

		address.sun_family = AF_UNIX;
		snprintf(address.sun_path, 128, "/dev/gpio%d", gpio);

		if(connect(fd[gpio], (struct sockaddr *)&address, sizeof(struct sockaddr_un)) != 0) {
			logprintf(LOG_ERR, "connect");
			return -1;
		}
	}

	return 0;
}

void wiringXIOCTLCallback(int (*callback)(int fd, unsigned long req, void *cmd)) {
	ioctl_callback = callback;
}

int ioctl(int fd, unsigned long req, void *cmd) {
	if(ioctl_callback != NULL) {
		return ioctl_callback(fd, req, cmd);
	} else {
		return -1;
	}
}

int wiringXGC(void) {
	int *(*real)(void) = dlsym(RTLD_NEXT, "wiringXGC");
	if(NULL == real) {
		fprintf(stderr, "dlsym");
	}
	real();
	int i = 0;
	for(i=0;i<255;i++) {
		if(fd[i] > 0) {
			close(fd[i]);
		}
	}
  return 0;
}

int wiringXISR(int gpio, int mode) {
	if(gpio == 2) {
		return -1;
	}
	if(fd[gpio] == -1) {
		return pinMode(gpio, mode);
	} else {
		return 0;
	}
}

test.c

#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <wiringx.h>
#include <sys/ioctl.h>
#ifndef __FreeBSD__
	#include <linux/spi/spidev.h>
#endif

#include "libuv/uv.h"

#define BUFFER_SIZE 1024

void wiringXIOCTLCallback(int (*callback)(int fd, unsigned long req, void *cmd));

int ioctl_callback(int fd, unsigned long req, void *cmd) {
	printf("%d %lu\n", fd, req);
	return 0;
}

static void close_cb(uv_handle_t *handle) {
	free(handle);
}

static void walk_cb(uv_handle_t *handle, void *arg) {
	if(!uv_is_closing(handle)) {
		uv_close(handle, close_cb);
	}
}

void open_cb(uv_fs_t *req) {
	wiringXIOCTLCallback(ioctl_callback);
	wiringXSPISetup(0, 2500000UL);
}

int main(void) {
	uv_fs_t *req = malloc(sizeof(uv_fs_t));

	uv_fs_open(uv_default_loop(), req, "/dev/spidev0.0", O_CREAT, O_RDWR, open_cb);

	uv_run(uv_default_loop(), UV_RUN_DEFAULT);
	uv_walk(uv_default_loop(), walk_cb, NULL);
	uv_run(uv_default_loop(), UV_RUN_ONCE);

	while(uv_loop_close(uv_default_loop()) == UV_EBUSY) {
		uv_run(uv_default_loop(), UV_RUN_ONCE);
	}

	return 1;
}

Makefile

# inside the cmake build folder.
gcc -c -Wall -Werror -fpic libgpio.c -I../libs
gcc -shared -o libgpio.so libgpio.o
gcc -o test test.c ./libpilight.so ./libgpio.so -I../libs -lwiringx; LD_PRELOAD=./libgpio.so ./test

What it does:

  1. First create a file called /dev/spidev0.* so it can be opened by wiringxSPISetup.
  2. In libgpio.c add a custom unittest function called wiringXIOCTLCallback which allows us to create a custom callback for the ioctl commands.
  3. In libgpio.c override the ioctl command with a custom one which triggers the callback
  4. Inside the unittest, use the callback to validate the ioctl write commands or to pass data to the ioctl read commands.
  5. Use LD_PRELOAD to actually override those functions.

Can you continue with this snippet?

@Phunkafizer
Copy link
Author

Phunkafizer commented Jan 23, 2019 via email

@CurlyMoo
Copy link
Contributor

Ofc

@Phunkafizer
Copy link
Author

I force-pushed here:

https://github.com/Phunkafizer/pilight/tree/rewrite

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Jan 29, 2019

If you aren't using FREE when you are using MALLOC my memory tracker can't track the memory allocations and collections.

@Phunkafizer
Copy link
Author

I just transfered basically from hardware_433gpio_receive.c, but it seems to work w/o FREE? I added a FREE to the end of my code (forced push again). The Warning disappeared but I still get the double free issue.

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Jan 29, 2019

free is not using my memory debugger. Using xfree at the end therefor misses the free calls. FREE wraps free and does inform my debugger.

Also, i couldn't confirm the double free issue.

@Phunkafizer
Copy link
Author

You did run my commit? On arm? Did you get a "TRAIN MATCH!" output?

free is not using my memory debugger. Using xfree at the end therefor misses the free calls. FREE wraps free and does inform my debugger.

Also, i couldn't confirm the double free issue.

This means I should not use xfree?

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Jan 29, 2019

You did run my commit? On arm? Did you get a "TRAIN MATCH!" output?

I did and i got it.

This means I should not use xfree?

You should, because it checks your memory allocations for you. It prevents you having to use valgrind all the time.

@Phunkafizer
Copy link
Author

I rebased my rewrite branch today to your current one. I made some adjustments to make it compile. After running I get a seg fault at

options_add(&raspyrfm->options, "f", "frequency", OPTION_HAS_VALUE, DEVICES_VALUE, JSON_NUMBER, NULL, "^[0-9.]+$");

Did you change something in the deep?

My latest code is again on my rewrite branch:
https://github.com/Phunkafizer/pilight/tree/rewrite

@CurlyMoo
Copy link
Contributor

No, didn't change anything there. What does gdb say?

@Phunkafizer
Copy link
Author

For some reason gdb does not Work in my Crosssetup. Is there any other way to findout more?

@CurlyMoo
Copy link
Contributor

valgrind?

@Phunkafizer
Copy link
Author

For some reason gdb does not Work in my Crosssetup. Is there any other way to findout more?

@Phunkafizer
Copy link
Author

Phunkafizer commented Feb 10, 2019

I managed to run GDB directly on the PI in a terminal. When running pilight-unittest with test_hardware_raspyrfm_receive I get

0xb677cd34 in options_add (opt=0xc, id=0xb6834f54 "f", name=0xb6834f30 "frequency", argtype=2, conftype=3, vartype=8, def=0x0,
    mask=0xb6834f58 "^[0-9.]+$") at /home/pi/dev/pilight/libs/pilight/core/options.c:506
506             } else if(options_get_name(*opt, id, &ctmp) == 0) {
(gdb)
Unable to fetch general registers.: No such process.
Unable to fetch general registers.: No such process.
Unable to fetch general registers.: No such process.
(gdb) [Thread 0xb4b47470 (LWP 30869) exited]
[Thread 0xb5347470 (LWP 30868) exited]
[Thread 0xb5b47470 (LWP 30867) exited]
[Thread 0xb6347470 (LWP 30866) exited]

Program terminated with signal SIGSEGV, Segmentation fault.

@CurlyMoo
Copy link
Contributor

I have to dig into this.

@CurlyMoo
Copy link
Contributor

It was indeed due to some serieus changes i did porting the hardware modules to lua. I would advice you to stay on 2ffea92 for now.

@Phunkafizer
Copy link
Author

I rebased my feature on your suggested commit. Somewhing weired is going on now. When running the pilight-daemon, execution stucks in the while loop (100% CPU) here:

https://github.com/Phunkafizer/pilight/blob/1e7667152fd4cfaf7a1cedc0230998731e086813/daemon.c#L1619

This call
int c = options_parse1(&options, argc, argv, 1, &args, NULL);

always returns 0, because options.c says

int options_parse1(struct options_t **opt, int argc, char **argv, int error_check, char **optarg, char **ret) { return 0; }

Am I missing something?

@CurlyMoo
Copy link
Contributor

The daemon doesn't work. Only pilight-unittest

@Phunkafizer
Copy link
Author

I don't get it really. I use the same way as in hardware_433gpio_receive.c: MALLOC for the client_req, and xfree at the end. I'm getting:

WARNING: unfreed pointer (0x2017e48) in /home/pi/dev/pilight/tests/hardware_raspyrfm_receive.c at line #147 *** Error in ./pilight-unittest': double free or corruption (!prev): 0x02017e48 ***
Aborted`

Unfreed and double free at the same time?

@CurlyMoo
Copy link
Contributor

As i said before, you need to use FREE instead of free here:
https://github.com/Phunkafizer/pilight/blob/rewrite/tests/hardware_raspyrfm_receive.c#L108

@Phunkafizer
Copy link
Author

Phunkafizer commented Feb 16, 2019

I'm doing so:

static void close_cb(uv_handle_t *handle) { FREE(handle); }

Sorry was on the wrong branch, working now! I'll go on with the unittest!

@Phunkafizer
Copy link
Author

After understanding uvlib better unittest is work for send & receive now. Thank you very much for your help.

@CurlyMoo
Copy link
Contributor

You're welcome. Now these unittests work, we can work towards porting the whole thing into a Lua module 😄

I will do the initial work based on your unittest, when i'm done, you can test the whole thing with the real hardware.

@Phunkafizer
Copy link
Author

Yes, time has passed quickly! I tried all available unittests for the lua raspyrfm, they all have the same result:

There was 1 failure:

  1. test_lua_hardware_raspyrfm_param4: /home/pi/dev/pilight/tests/lua_hardware_raspyrfm.c:474: expected <0> but was <-13>

!!!FAILURES!!!
Runs: 1 Passes: 0 Fails: 1

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Nov 11, 2019

They all run fine here:

root@dev-ubuntu:/home/pilight/source/raspyrfm/build1# ./pilight-unittest test_lua_hardware_raspyrfm_send
[ test_lua_hardware_raspyrfm_send                  ]
.

OK (1 test)

root@dev-ubuntu:/home/pilight/source/raspyrfm/build1# ./pilight-unittest test_lua_hardware_raspyrfm_receive
[ test_lua_hardware_raspyrfm_receive               ]
.

OK (1 test)

root@dev-ubuntu:/home/pilight/source/raspyrfm/build1# ./pilight-unittest test_lua_hardware_raspyrfm_param1
[ test_lua_hardware_raspyrfm_param1                ]
.

OK (1 test)

root@dev-ubuntu:/home/pilight/source/raspyrfm/build1# ./pilight-unittest test_lua_hardware_raspyrfm_param2
[ test_lua_hardware_raspyrfm_param2                ]
.

OK (1 test)

root@dev-ubuntu:/home/pilight/source/raspyrfm/build1# ./pilight-unittest test_lua_hardware_raspyrfm_param3
[ test_lua_hardware_raspyrfm_param3                ]
.

OK (1 test)

root@dev-ubuntu:/home/pilight/source/raspyrfm/build1# ./pilight-unittest test_lua_hardware_raspyrfm_param4
[ test_lua_hardware_raspyrfm_param4                ]
.

OK (1 test)

Did you compile it properly?

cmake -DPILIGHT_UNITTEST=1 \
        -DCOVERALLS=ON \
        -DCMAKE_BUILD_TYPE=Debug ..

@Phunkafizer
Copy link
Author

I started from scratch again (branch raspyrfm). When compiling I get

/home/pi/dev/pilight/inc/../libs/pilight/core/mem.h:38:18: error: called object ‘free’ is not a function or function pointer
  #define FREE(a) free(a),(a)=NULL
                  ^
/home/pi/dev/pilight/libs/pilight/lua_c/lua.c:1592:7: note: in expansion of macro ‘FREE’
       FREE(lua_state[i].gc.list[x]);
       ^~~~
/home/pi/dev/pilight/libs/pilight/lua_c/lua.c:1582:20: note: declared here
  int i = 0, x = 0, free = 1;
                    ^~~~
In file included from /home/pi/dev/pilight/inc/../libs/pilight/core/pilight.h:45:0,
                 from /home/pi/dev/pilight/inc/../libs/pilight/core/common.h:32,
                 from /home/pi/dev/pilight/libs/pilight/lua_c/lua.h:16,
                 from /home/pi/dev/pilight/libs/pilight/lua_c/lua.c:26:
/home/pi/dev/pilight/inc/../libs/pilight/core/mem.h:38:25: warning: left-hand operand of comma expression has no effect [-Wunused-value]
  #define FREE(a) free(a),(a)=NULL
                         ^
/home/pi/dev/pilight/libs/pilight/lua_c/lua.c:1592:7: note: in expansion of macro ‘FREE’
       FREE(lua_state[i].gc.list[x]);
       ^~~~

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Nov 11, 2019

There must be something wrong with your dev machien because free is one of the most basic C functions there is.

What's the environment you are working in? Raspberry Pi or cross-compilation?

@Phunkafizer
Copy link
Author

After running cmake again it worked fine, but getting the same unittestresults as before...
My platform is raspberry.

@CurlyMoo
Copy link
Contributor

raspberry

Which one, because the performance difference between versions is quite big.

@Phunkafizer
Copy link
Author

It's the Raspberry 3B. I'll try with a second PI and a fresh SD card.

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