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 RPI-RF-MOD/HM-MOD-RPI-PCB for HomeMatic/homematicIP communication #1266

Merged
merged 26 commits into from
Apr 7, 2021

Conversation

jens-maus
Copy link
Contributor

@jens-maus jens-maus commented Mar 10, 2021

Abstract
In its' current form, the native HA provided HomeMatic CCU add-on in combination with HAos is largely limited in terms of functionality, simply because HAos does not come with the necessary kernel and software requirements to utilize the dual-stack HomeMatic/homematicIP communication channels provided by recent firmware versions of the RPI-RF-MOD or HM-MOD-RPI-PCB rf modules developed by eQ3 / ELV, which are - however - required for seamless integration of modern homematic devices.

cf. home-assistant/addons#1751
cf. home-assistant/addons#1796
cf. https://community.home-assistant.io/t/can-t-get-homematic-addon-working/275528/11

Therefore, this pull request proposes integration of changes to bring up direct support for the RPI-RF-MOD and HM-MOD-RPI-PCB GPIO HATs to HAos while at the same time leaving a default HAos system unchanged if no such hardware is used or installed.

Furthermore, in a separate HomeMatic/homematicIP-related endeavour a new RaspberryMatic CCU HomeAssistant add-on has been recently developed and published as an early beta version. This version comes with basic functionality, but also still lacks the fundamental functionality of being able to use the RPI-RF-MOD / HM-MOD-RPI-PCB rf modules for a full-fledged "HomeMatic CCU" functionality because the underlying operating system (HAos) lacks the necessary drivers.

Proposed Changes

  1. Integrate new buildroot packages (eq3_char_loop, generic_raw_uart ) for building+installing all necessary kernel modules for low-latency HomeMatic/homematicIP dual-stack communication required when using a RPI-RF-MOD / HM-MOD-RPI-PCB either directly on the GPIO bus or by using USB/LAN adapter PCBs (HB-RF-USB-2, HB-RF-ETH).
  2. Integrate new buildroot package (rpi-rf-mod) which generates RaspberryPi and ASUS Tinkerboard device tree overlays for the GPIO pins used by the RPI-RF-MOD / HM-MOD-RPI-PCB (including pins to drive LEDs and module reset).
  3. Modify hassos-hook.sh hook for RaspberryPi and Tinkerboard to install the new device tree overlays to /mnt/boot/overlays.
  4. Modify raspberrypi/boot-env.txt by adding a commented section to config.txton how a user can manually enable direct GPIO support for RPI-RF-MOD / HM-MOD-RPI-PCB
  5. Enable CONFIG_GPIOLIB and CONFIG_GPIO_SYSFS kernel config options in device-support.config providing the necessary GPIO kernel infrastructure for the above mentioned kernel modules and allowing to set GPIO pins via sysfs.
  6. Modified all platform defconfig files to enable all new above mentioned buildroot packages.

Technology Preview
The proposed changes has already been integrated and tested by a group of RaspberryMatic CCU add-on beta testers. For this purpose, developer builds of HAos were generated and are publically available at

https://github.com/jens-maus/operating-system/releases/tag/rm-testversion

Discussions and latest state-of-affairs on the integration and testing of the proposed changes in the context of the RaspberryMatic CCU add-on can be found here:

jens-maus/RaspberryMatic#1087

Technical Dependencies
The changes proposed here are largely based on functionality and knowhow gained during development of the OSS RaspberryMatic project, which - similar to HAos - is based on buildroot and provides functionality to build an own HomeMatic CCU central on RaspberryPi, Tinkerboard system or even on virtual OVA environments. Thus, the new proposed buildroot packages for kernel driver integration have been largely borrowed from the respective buildroot-external section in the RaspberryMatic project (see https://github.com/jens-maus/RaspberryMatic/tree/master/buildroot-external/package). Thus, future updates on the necessary kernel drivers+dependencies can be easily ported via additional pull requests.

Limitations
The following known limitation exist, which will be, however, addressed with further PullRequests in future:

  • The ODROID platform integrations are currently not supported because of the missing device tree overlays

Licensing Dependencies

@alexreinert
Copy link
Contributor

I'm the author of the generic_raw_uart kernel modules. As Jens included them in a way, that is not the supposed way, I would prefer, that this PR is not merged. I mentioned this to Jens a month ago, but he did not react on it.

@jens-maus
Copy link
Contributor Author

jens-maus commented Mar 11, 2021

@alexreinert When you initially brought up your concerns I immediately replied and explicitly asked you to raise your technical arguments and contribute to the development of the HomeAssistant integration of the RaspberryMatic add-on before I am going to submit a PR. Neither did you contact me directly nor contribute in any way, but simply initially stated that this is not the "right" way of doing it. However, I still don't see any real technical reasons why the proposed changes in this PR (which also integrates your generic_raw_uart) might not be done in a sensible or technically valid way or might be harmful for any component of HAos. All previous testers and reviewers of these changes (cf. jens-maus/RaspberryMatic#1087) didn't raise any technical reasons why this should be done in another way or how this should be done different. That you are using these kernel drivers in your own HomeMatic-related endeavours (piVCCU, debmatic) in a different way is perfectly valid but might not be directly valid in the present context of HomeAssistant or the RaspberryMatic HA add-on.

So please, state what you think might be the "supposed way" of integrating your kernel modules and what you think might be wrong in the way I am proposing it and where you feel my changes might do harm to the HAos eco system. Then I would be happy to start a technical discussion with potentially integrating changes that you think might be more optimal or smooth than the proposed way in this PR.

@alexreinert
Copy link
Contributor

You removed my conserns together with the branch, there I write it down, there were some technical arguments. Either way:
In this PR you added the stuff like you use it in RaspberryMatic and not in the way like it is done in the reference implementation. Therefore you did some patches that will force any other user of the generic-raw-uart to use the kernel modules in your way and not in the way of the reference implemenation. The supposed way here is to work without the RaspberryMatic specific patches.
Adding the hb-rf-usb and hb-rf-usb-2 kernel modules is a breaking change for user of this pcbs using the already existing Homematic HA addon.
And a last thing you never mentioned: Who will be the maintainer of the new packages? Are you willing to support and fix all HASS related issues?

@jens-maus
Copy link
Contributor Author

jens-maus commented Mar 11, 2021

You removed my conserns together with the branch, there I write it down, there were some technical arguments.

Sorry. For preparing this PR I had to redo my fork and cleaned up the patches which unfortunately wiped our previous conversation on that matter.

In this PR you added the stuff like you use it in RaspberryMatic and not in the way like it is done in the reference implementation. Therefore you did some patches that will force any other user of the generic-raw-uart to use the kernel modules in your way and not in the way of the reference implemenation. The supposed way here is to work without the RaspberryMatic specific patches.

To be more specific, you are talking about the following two minor changes, right?

https://github.com/home-assistant/operating-system/pull/1266/files#diff-595139ce2051b540bc46854789004a236c36e642328cbc82a8f56069965707d6
https://github.com/home-assistant/operating-system/pull/1266/files#diff-5f033ae569cc54160eb70b019f2c5af818f8419083c84f6bbb0206759a902074

So namely, the change of disabling the pivccu,reset_pin and other pivccu,XXXX device tree overlay entries for defining the LED GPIO pins.

However, there are reasons behind these changes:

  1. Making sure the GPIOs are not locked by the system because they were defined in these device tree overlays and thus cannot be assigned in the rpi-rf-mod device tree overlay or via sysfs.
  2. To be able to use a different reset GPIO pin because the HM-MOD-RPI-PCB uses a different GPIO (GPIO18) for reset than the RPI-RF-MOD (GPIO19) and based on the used RF module type one has to reset the module differently.

If you could assist, I would of course try to integrate some more changes that might comply more to your "reference implementation" and also then adapt the way the RaspberryMatic add-on is using these device tree overlays. So what do you think might be required here?

But in terms of flexibility I still think that my approach is not only technically valid but also provides more flexibility than fixing these pins explicitly in these device tree overlays and doesn't immediately pose a problem here. And in terms of "other users" I am still unsure which other users are you talking about? We are talking here only about the native "HomeMatic CCU" and the new "RaspberryMatic CCU" add-on IMHO.

Adding the hb-rf-usb and hb-rf-usb-2 kernel modules is a breaking change for user of this pcbs using the already existing Homematic HA addon.

Which "breaking changes" are you talking about? Actually, I don't know of any HAos users using a HB-RF-USB/HB-RF-USB-2 with the native "HomeMatic CCU" add-on and why my proposed changes should pose any breaking change for them? The "old" Homematic CCU add-on is AFAIK only compatible to the HM-MOD-RPI-PCB with firmware 1.4.1 or for being used with a HmIP-RFUSB (cf. https://github.com/home-assistant/addons/tree/master/homematic). The add-on also does not come with own kernel drivers for the HB-RF-USB/HB-RF-USB-2. So please explain a bit more.

And a last thing you never mentioned: Who will be the maintainer of the new packages? Are you willing to support and fix all HASS related issues?

Yes, I am perfectly willing (and actually missed to propose it above) to take the role of the maintainer for all the new integrated buildroot packages including the generic_raw_uart one, if the HAos devs would allow me to take that role.

@alexreinert
Copy link
Contributor

So namely, the change of disabling the pivccu,reset_pin and other pivccu,XXXX device tree overlay entries for defining the LED GPIO pins.

However, there are reasons behind these changes:

1. Making sure the GPIOs are not locked by the system because they were defined in these device tree overlays and thus cannot be assigned in the [rpi-rf-mod](https://github.com/home-assistant/operating-system/pull/1266/files#diff-d3a862b3485130160fa6e6eabaa09132f31b710f1b04c3215290c07a1cb2f00a) device tree overlay or via sysfs.

The way how the pins are used in the device tree and in the generic-raw-uart kernel module are not locking them. They are only referenced because the GPIO numbers are not absolutly stable, they can change from kernel version to kernel version (like it was on the tinkerboard shortly after the first release). The way in the reference implementation is using the way how device tree should be used, as an abstraction layer to the underlying hardware, so that the software layers above do not need to implement hardware specific parts. You can see that in piVCCU and debmatic, which supports more than 30 different single board computers without the need of special treatment inside the scripts just by using different device tree overlays.

2. To be able to use a different reset GPIO pin because the `HM-MOD-RPI-PCB` uses a different GPIO (GPIO18) for reset than the `RPI-RF-MOD` (GPIO19) and based on the used RF module type one has to reset the module differently.

Even this the unpatched device tree overlay you can use your custom way of RaspberryMatic. And as we discussed some time ago: Resetting the RPI-RF-MOD using the GPIO pin is not absolutly needed as you can do a reset by switching to bootloader and back to app mode.
But a change in the kernel modules to support the different reset pins is already in the pipeline using the same approach as the current solution (using device tree for hardware specific definitions)

If you could assist, I would of course try to integrate some more changes that might comply more to your "reference implementation" and also then adapt the way the RaspberryMatic add-on is using these device tree overlays. So what do you think might be required here?

But in terms of flexibility I still think that my approach is not only technically valid but also provides more flexibility than fixing these pins explicitly in these device tree overlays and doesn't immediately pose a problem here. And in terms of "other users" I am still unsure which other users are you talking about? We are talking here only about the native "HomeMatic CCU" and the new "RaspberryMatic CCU" add-on IMHO.

Currently there is no other system, right. But you try to add changes outside your addon, imho they should be generic and not RaspberryMatic specific.

Adding the hb-rf-usb and hb-rf-usb-2 kernel modules is a breaking change for user of this pcbs using the already existing Homematic HA addon.

Which "breaking changes" are you talking about? Actually, I don't know of any HAos users using a HB-RF-USB/HB-RF-USB-2 with the native "HomeMatic CCU" add-on and why my proposed changes should pose any breaking change for them? The "old" Homematic CCU add-on is AFAIK only compatible to the HM-MOD-RPI-PCB with firmware 1.4.1 or for being used with a HmIP-RFUSB (cf. https://github.com/home-assistant/addons/tree/master/homematic). The add-on also does not come with own kernel drivers for the HB-RF-USB/HB-RF-USB-2. So please explain a bit more.

You can use the combination HB-RF-USB(-2)/HM-MOD-RPI-PCB with the BidCos only firmware without own kernel drivers just using the existing default kernel modules just using an udev rule. And I know at least two persons, which are using that.

Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

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

This is quite some kernel code which gets loaded here, and I am not a huge fan of out-of-tree code. That said, as long as things only get loaded when manually enabled/the hardware is actually used (which from what I understand is the case), I am ok with merging this.

buildroot-external/package/rx8130-rtc/Config.in Outdated Show resolved Hide resolved
home-assistant#1266 (comment)
Now a new config option (BR2_PACKAGE_GENERIC_RAW_UART_DTS) have to be
used together with selection of the respective target platform so that
the generic_raw_uart package compiles the selected device tree overlay.
@jens-maus
Copy link
Contributor Author

@alexreinert

The way how the pins are used in the device tree and in the generic-raw-uart kernel module are not locking them. They are only referenced because the GPIO numbers are not absolutly stable, they can change from kernel version to kernel version (like it was on the tinkerboard shortly after the first release). The way in the reference implementation is using the way how device tree should be used, as an abstraction layer to the underlying hardware, so that the software layers above do not need to implement hardware specific parts.

Ok, I will see if I can test it again because I still recall that there was a locking issue back when I added the patches against the generic_raw_uart and I will try to work out a potential solution ASAP. A said already, if you could assist by submitting changes yourself we could potentially speed it up a lot, I guess.

[...]
But a change in the kernel modules to support the different reset pins is already in the pipeline using the same approach as the current solution (using device tree for hardware specific definitions)

Would be really good if you could implement that change in your kernel modules ASAP so that we could merge them in future for a IMHO smoother support for the HM-MOD-RPI-PCB.

But in terms of flexibility I still think that my approach is not only technically valid but also provides more flexibility than fixing these pins explicitly in these device tree overlays and doesn't immediately pose a problem here. And in terms of "other users" I am still unsure which other users are you talking about? We are talking here only about the native "HomeMatic CCU" and the new "RaspberryMatic CCU" add-on IMHO.

Currently there is no other system, right. But you try to add changes outside your addon, imho they should be generic and not RaspberryMatic specific.

I agree, if there is a way to do this generic easily I am all fine with it. But as said, I do recall that there were good reasons for these patches back then. If these reasons still exist or are still valid in the context of Home Assistant have to be seen, thought. And still, I highly doubt that there will be any further homematic-related add-on coming up anytime soon. And if so, we could propose further changes with subsequent PRs. So even if we would not be able to create a 100% generic solution in the first iteration, the current set of changes already pose a clear improvement IMHO.

Which "breaking changes" are you talking about? Actually, I don't know of any HAos users using a HB-RF-USB/HB-RF-USB-2 with the native "HomeMatic CCU" add-on and why my proposed changes should pose any breaking change for them? The "old" Homematic CCU add-on is AFAIK only compatible to the HM-MOD-RPI-PCB with firmware 1.4.1 or for being used with a HmIP-RFUSB (cf. https://github.com/home-assistant/addons/tree/master/homematic). The add-on also does not come with own kernel drivers for the HB-RF-USB/HB-RF-USB-2. So please explain a bit more.

You can use the combination HB-RF-USB(-2)/HM-MOD-RPI-PCB with the BidCos only firmware without own kernel drivers just using the existing default kernel modules just using an udev rule. And I know at least two persons, which are using that.

Well, but this is not an official feature of the HomeMatic CCU add-on nor is it documented in any way anywhere. Thus, I wouldn't really count it a breaking change. And even if, IMHO the benefit of adding general support for the HB-RF-USB(-2) adapter pcbs overrules this undocumented use.

mainline rtc-ds1307 kernel module which in fact also comes with supercap
charging functionality in recent 5.10.x kernels. Thus there does not
seem to be a reason for the seperate kernel module anymore which is used
on a RPI-RF-MOD. This refs home-assistant#1266 (review)
and replaced it by dedciated TARGET_RPI/TINKER config options to match
the way this is done in other buildroot packages in HAos.
reset_pin,red/greeen/blue_pin pointer to zero. In addition, the rpi-rf-mod
package is now a meta package taking care to define all required
dependencies in its Config.in.
@jens-maus
Copy link
Contributor Author

@agners
Please note, that I slightly reworked this PR now. Apart from having integrated the necessary changes according to suggestions of @alexreinert, I also tried to simplify the whole changeset by reducing the number of defconfig entries being necessary in the individual platform defconfig files. Now, just the BR2_PACKAGE_RPI_RF_MOD (and some additional XXX_DTS) config option is required and the rest is then managed via dependency definitions. Thus, the rpi-rf-mod buildroot package has now a meta package character automatically adding the other packages as needed.

This, together with the other previous changes (e.g. replacement of the dedicated rtc-rx8130 package with 5.10.x mainline rx8130 use, removal of BR2_PACKAGE_HASSIO_MACHINE uses) should hopefully fulfil your requested changes. Thus, looking forward to get it quickly reviewed again from your side and then potentially merged anytime soon.

Please note, that the only thing that is still missing is the device tree overlay support for the tinkerboard platform. I saw that you already integrated a newer u-boot version. Hopefully this version will already provide the necessary changes to support device tree overlays on the tinkerboard, so that just a change of the u-boot bootscript is required to allow u-boot on the tinkerboard to load the rpi-rf-mod device tree overlay similar to the RaspberryPi platform. However, I would prefer to address this in one of the next PRs I will then submit afterwards.

package to use 'alt_reset_pin' device tree node entry to implement
different reset pins for RPI-RF-MOD and HM-MOD-RPI-PCB.
bootEnv.txt loading support to uboot boot script.
@jens-maus
Copy link
Contributor Author

@agners FYI: I just committed the last missing bit regarding this PR. I added device tree overlay support for the ASUS Tinkerboard and adapted its u-boot boot script to load /boot/bootEnv.txt if it exists so that users of a tinkerboard can manually enable loading of the ´rpi-rf-mod.dtbo` overlay file if they want to (similar to how this is done with the RaspberryPi).

Looking forward to seeing this PR merged in the near future.

Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

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

Looks quite good I would say. Still don't like downstream kernel modules of course :) It will be a pain to upgrade kernels etc. But its fairly separated so we can disable them/fix them in separate pull request when we have troubles on kernel updates.

Are there any plans to make things work with upstream kernels?

@alexreinert
Copy link
Contributor

Maybe the Device Tree Overlays should be separated. Even when a HM-MOD-RPI-PCB is used, the GPIO pins 36,38,40 are blocked. This could be fixed by separating the stuff in two DTOs, one for the generic_raw_uart and one for the RPI-RF-MOD gpio-led.

@jens-maus
Copy link
Contributor Author

Looks quite good I would say. Still don't like downstream kernel modules of course :) It will be a pain to upgrade kernels etc. But its fairly separated so we can disable them/fix them in separate pull request when we have troubles on kernel updates.

As said before. I would volunteer as a maintainer of all these changes and these new Buildroot patches and as I am regularly updating my own project (RaspberryMatic), which is also buildroot based and is using the same patches/functionality, there should be low risk that we end up in this situation. But yes, if this might come up you can disable all these packages as this functionality is anyway just optional.

Are there any plans to make things work with upstream kernels?

I doubt that there is any chance to get these kernel modules accepted/integrated in upstream kernels. First of all, these GPIO RF modules are quite unique and so are the kernel modules they unfortunately require. And secondly, these hardware rf HATs are mainly just popular in Germany/Europe. So chances are quite low that they ever will be integrated by the upstream linux devs. But as said before. As soon as these HATs or HomeMatic/homematicIP might get obsolete or not vendor supported anymore, it should be fairly easy to remove all this again with only minor effort.

eq-3/occu repository directly and patch all required changes to get
version 1.1 rather than directly maintaining its sources downstream.
@jens-maus
Copy link
Contributor Author

@agners
As you might have spotted, I have just updated this PR according to your latest requested changes. Namely:

  1. AddedCONFIG_OF_LIBFDT_OVERLAY to the tinkerboard uboot.config rather than patching the Kconfig file.
  2. modified the eq3_char_loop as well as the rpi-rf-mod buildroot package to use the upstream sources rather than adding them downstream here in the PR.

Thus, I do hope that now with having fulfilled your change request, this PR should be finally clean enough (e.g. no downstream sources anymore) to get it merged ASAP. Any further adaptions to this functionality we could then IMHO tune via subsequent PRs as specific requirements from users might come up.

@agners
Copy link
Member

agners commented Apr 7, 2021

@jens-maus looks good to me now, thanks for your work!

@jens-maus
Copy link
Contributor Author

@jens-maus looks good to me now, thanks for your work!

Thanks! So the last question here is: What is the ETA for seeing this integrated in an upcoming official HAos release?

@agners
Copy link
Member

agners commented Apr 7, 2021

@jens-maus I renamed the boot config file from bootEnv.txt to haos-config.txt. This aligns the naming with Raspberry Pi (config.txt), but still being different since its our U-Boot scripts which read that (compared to Raspberry Pi boot loader for config.txt). Hope this is fine by you.

One thing I still wondered: You also enable BR2_PACKAGE_RPI_RF_MOD for generic-x86-64 and odroid boards. Is this functional without overlays etc?

@jens-maus
Copy link
Contributor Author

jens-maus commented Apr 7, 2021

@jens-maus I renamed the boot config file from bootEnv.txt to haos-config.txt. This aligns the naming with Raspberry Pi (config.txt), but still being different since its our U-Boot scripts which read that (compared to Raspberry Pi boot loader for config.txt). Hope this is fine by you.

I don't really care. Thought I use bootEnv.txt in my own project. Thus haos-config.txt seems to be somewhat too project specific IMHO. I would probably (if you really don't like bootEnv.txt) then just name it config.txt like with the RaspberryPi even thought U-boot loads it but then its purpose and usage is clear and consistent with the raspberrypi platform

One thing I still wondered: You also enable BR2_PACKAGE_RPI_RF_MOD for generic-x86-64 and odroid boards. Is this functional without overlays etc?

Yes, you have to keep in mind that there are solutions to use a RPI-RF-MOD without an onboard GPIO. Namely by using the HB-RF-USB and HB-RF-ETH pcb adapters and this package will then also compile these necessary kernel modules and integrate them. So yes, they are required even thought there are no device tree overlays or GPIO ports available on a platform.

@agners agners added board/generic-x86-64 Generic x86-64 Boards (like Intel NUC) board/odroid Hardkernel's ODROID Boards board/ova Open Virtual Appliance (Virtual Machine) board/raspberrypi Raspberry Pi Boards board/tinker ASUS' Tinker Boards labels Apr 7, 2021
@agners agners merged commit 0eef647 into home-assistant:dev Apr 7, 2021
@jens-maus
Copy link
Contributor Author

@agners Final thanks for having integrated this PR. This will be definitely highly appreciated by the HomeMatic community, I guess.

Until this will be part of one of the next official releases, can you please trigger a new haos development build with this PR integrated so that the current beta testers of this functionality can use these development builds rather than the manual builds I provided them some time ago? And what about an ETA for landing of this functionality in an official stable release?

@agners
Copy link
Member

agners commented Apr 7, 2021

I am doing some tests with the last night build right now, I'll start a development release somewhat later today.

OS release 6 starts to get where it should be: U-Boot, Kernel and Buildroot are updated, and my initial tests look good. There is still some more testing planned, and we need to have OS Agent somewhat settled. But I don't think we have big road blocks, so hopefully sometime mid/end April.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
board/generic-x86-64 Generic x86-64 Boards (like Intel NUC) board/odroid Hardkernel's ODROID Boards board/ova Open Virtual Appliance (Virtual Machine) board/raspberrypi Raspberry Pi Boards board/tinker ASUS' Tinker Boards cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants