XKNX / xknx

XKNX - A KNX library written in Python
http://xknx.io/
MIT License
282 stars 101 forks source link

Add support for USB Knx Interfaces #581

Open farmio opened 3 years ago

farmio commented 3 years ago

It would be nice to support USB interfaces in addition to KNX/IP. Next to supporting users that don't own IP Interfaces Xknx could be used as USB - IP bridge in the future.

See KNX specification 9 - 3: Basic System and Components - Couplers §3 KNX USB Interface

farmio commented 1 year ago

We should probably implement a Device Feature Get - Response cycle on start - raising an error if the device doesn't support CEMI or do a Device Feature Set if it supports more than CEMI.

Currently the thread-encapsulation is not great... we call xknx.cemi_handler.handle_cemi_frame (which calls xknx.telegrams.put_nowait(telegram) or xknx.management.process(telegram)) from the receive thread - which should be handled in main thread. For a proof of concept, I don't think this is a problem, but in production code this should probably be avoided.

Also, do you think it is possible to do send and receive in a single thread? I could imagine something like this to work (untested pseudo code)

self._usb_device.set_nonblocking(True)
while self._is_active.is_set():
    if self.out_queue.qsize(): # Return the approximate size of the queue (not reliable!). <- not reliable doesn't sound promising
        self.usb_device.write(self.out_queue.get_nowait())  # maybe `try: except Empty:` - maybe even without qsize test
    usb_data = self._usb_device.read()
    if usb_data:
        self._knx_to_telegram.process(usb_data)
kistlin commented 1 year ago

Implementing a feature check makes sense.

True asyncio.Queue is not thread safe. And in the CEMIHandler multiple thread contexts would be at work. With the proposed approach it evolves into timer based code, which I'm not a fan of. As soon as it is blocking at read, the thread cannot write. Now some time has to be chosen to timeout in the read to allow more writes.

Alternatives to check out could also be: awaitable loop.run_in_executor(executor, func, *args) For example whenever something was read and the executor returns the result, queue a new read task which contains the blocking read call. Write should just be a one off call.

Or janus

Mixed sync-async queue, supposed to be used for communicating between classic synchronous (threaded) code and asynchronous (in terms of [asyncio](https://docs.python.org/3/library/asyncio.html)) one.

https://stackoverflow.com/a/32894169/15862303

farmio commented 1 year ago

As soon as it is blocking at read

hence the set_nonblocking(1), or am I misunderstanding that? http://hidapi-d.dpldocs.info/hidapi.bindings.hid_set_nonblocking.html

As stated in the SO answer: Maybe a call to loop.call_soon_threadsafe could probably do it too. This would eliminate the need for another dependency. We already use that for IP - and I guess we could just use USB from the same class. https://github.com/XKNX/xknx/blob/e031588b42c87587df7c05e5a1d547484d84433b/xknx/io/knxip_interface.py#L514

kistlin commented 1 year ago

As soon as it is blocking at read

hence the set_nonblocking(1), or am I misunderstanding that? http://hidapi-d.dpldocs.info/hidapi.bindings.hid_set_nonblocking.html

True. But then the thread would be needlessly spinning.

Yes loop.call_soon_threadsafe sounds like the best option at this point. I agree to keep dependencies at a minimum.

kistlin commented 1 year ago

@farmio

it looks to me that my interface doesn't support CEMI, but only EMI1

Did you see that in a data sheet? I tried to quickly look it up but couldn't find anything. I compared the data exchange of my device when using ETS6. There it asks the interface which EMI version it understands and then configures it to use cEMI for example.

So maybe yours supports more too, it just defaults to EMI1?

farmio commented 1 year ago

I just looked up the datasheet of my Gira 107000 USB interface (here in german). As expected there is no word about EMI or CEMI 🫥

I should have been more verbose about that output of hidapitester i posted above. There I did a Device Feature Get Request with Service Identifier 01 - Supported EMI Type. See 09_03 Basic and System Components - Couplers v01.03.03 AS.pdf §3.5.3.2

Bildschirm­foto 2023-01-31 um 08 04 49

The response was a 0x01

Bildschirm­foto 2023-01-31 um 08 07 58

So it seems only EMI1 is supported with my device.

If multiple types were supported it would require to set the preferred type before further communication. See §3.5.3.3.2

martinmoravek commented 1 year ago

Hi all, is there any progress on supporting USB interfaces? thanks

farmio commented 1 year ago

@martinmoravek afaik, none apart from what you can read here in this issue or see here https://github.com/XKNX/xknx/tree/usb-interface-support

I don't have a cemi compatible usb interface, so I moved to different topics again for now.

Feel free to chime in if you like.

kistlin commented 1 year ago

Hi all, is there any progress on supporting USB interfaces? thanks

As to me, in recent times I did not invest time worth mentioning. And it is not planned in the near future. Maybe a few days within the next two to three months.

The current state is

The last part is holding me up currently. I tried to send what was mentioned by @farmio recently

If multiple types were supported it would require to set the preferred type before further communication. See §3.5.3.3.2

But it didn't seem to fix it. So I'm currently when I work on it comparing some captures.

After querying and setting the EMI type to use, I see a lot of M_PropRead.req/M_PropWrite.req. Does someone know how to read those messages? I see the message described in 03_06_03 EMI_IMI v01.03.03 AS.pdf but cannot make much sense out of Object Instance and Property ID and its data. Once I read a bit more on that there is maybe progress in having a stable communication from the start.

@martinmoravek if you are willing to test things or even develop that would be welcome. I only have one USB programmer and one KNX device. So no real variety to say with confidence if it works or not.

farmio commented 1 year ago

Since #1210 we should be able to decode these M_Prop messages, and there is also a module for decoding property ids 😃

kistlin commented 1 year ago

Ok thanks for the information.

And in the document it was right at the end of 03_06_03 EMI_IMI v01.03.03 AS.pdf in section 4.2.2 Generic management based on Interface Objects.

And while at it. The PID_COMM_MODE was by default FFh “no layer” on my device. Setting it to 00h Data Link Layer starts the communication. It can be checked with the Bus connection status (Bus Access Server Feature Service - Device Feature Get), which goes from 0 to 1.

So the next steps would be to integrate

and after that start normal communication.

martinmoravek commented 1 year ago

Hi all, is there any progress on supporting USB interfaces? thanks

As to me, in recent times I did not invest time worth mentioning. And it is not planned in the near future. Maybe a few days within the next two to three months.

The current state is

  • basic sending/receiving over USB works for me (:)) (receiving button presses and temperature values)
  • the initial communication with my USB device is not working, only after a couple of runs or using ES6 to monitor the bus

The last part is holding me up currently. I tried to send what was mentioned by @farmio recently

If multiple types were supported it would require to set the preferred type before further communication. See §3.5.3.3.2

But it didn't seem to fix it. So I'm currently when I work on it comparing some captures.

After querying and setting the EMI type to use, I see a lot of M_PropRead.req/M_PropWrite.req. Does someone know how to read those messages? I see the message described in 03_06_03 EMI_IMI v01.03.03 AS.pdf but cannot make much sense out of Object Instance and Property ID and its data. Once I read a bit more on that there is maybe progress in having a stable communication from the start.

@martinmoravek if you are willing to test things or even develop that would be welcome. I only have one USB programmer and one KNX device. So no real variety to say with confidence if it works or not.

@kistlin not sure if I am able to develop (never ever developed in python, only c/c++, sql), but testing is no problem (I have usb, ip as well as rs232 interface). with some advice and help to get the dev environment running I might try some dev as well

farmio commented 1 year ago

Setting up a dev env is straightforward. Provided you have a supported version of Python (>=3.9) installed, it should suffice to clone the repo, switch the branch and install dependencies (see root Readme).

RS232 is out of scope imho (it's not even supported by ETS anymore 😬).

kistlin commented 1 year ago

@martinmoravek No problem I can help you setup everyting. If you need help with a specific IDE/OS just tell me.

Additional steps before you install dependencies are creating a virtual environment. This prevents you from globally installing packages which will lead to version conflicts of packages, when you work on multiple projects.

To create one you can run python -m venv env in the root of the repository. (full path to Python if it is not in PATH)

And every time you launch a new terminal activate it with

You can also add as a second remote my fork https://github.com/kistlin/xknx. Works happens on usb-interface-support.

After you created your environment and installed the packages as mentioned in the README.md you can try and run

python examples/usb/example_telegram_monitor.py --filter "0/0/*"

and already check if you see something happening. (you might want to change KNX address to match your setup)

rnixx commented 8 months ago

Hi @farmio, what's the status of this issue and https://github.com/XKNX/xknx/tree/usb-interface-support? What's missing to get this done and upstream?

rnixx commented 8 months ago

I just tried to merge recent main to usb-interface-support here https://github.com/rnixx/xknx/tree/rnixx-usb-interface-support

knx_hid_helper_test.TestKNXtoCEMI.test_process fails now, looks like some new information from CEMIMessageCode is considered in the meantime. Either test frames have improper format or some hid related special handling is missing.

farmio commented 8 months ago

@rnixx Hi 👋!

What's missing to get this done and upstream?

Tbh I don't remember the exact last state of this, but you may read it in the previous comments. It didn't work on my interface at all since it doesn't support CEMI, so this would be needed to check and raise an exception (or translate CEMI to EMI1 if this is possible).

I don't know if anyone is still working on this.