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

Roja Flex support #1705

Closed
wants to merge 18 commits into from
Closed

Roja Flex support #1705

wants to merge 18 commits into from

Conversation

Hofyyy
Copy link
Contributor

@Hofyyy Hofyyy commented Apr 27, 2021

Supported Device:

  • Remote
  • Window Shutter

Extra feature:

  • Signal Cloner / Generator
    Could be activated in source.
    Take a real message, extract HOMEID and generate all possible command for all 15 channels:
    This is helpful if you what use an own bridge and you need the commands.

Review Feedback:

  • Copyright line missing?
  • Decoder doc should be just Markdown (no leading // ), first line the device name.
  • data_make() should be wrapped with /* clang-format off/on */

done

  • those short functions are preferred to be inline code (nothing in the decoder is reuseable...)

done. Two big functions are still there. Which is think helps.

preamble is best shortened to match truncated start, i.e. 0xaa 0xaa 0xd3...

Not done, because this will change all offsets, and in my tests this works great.
I understand the idea behind this point. Maybe I can rewrite it in a second round with
changed offsets.

  • you would usually check for the required minimum (bit)length, then truncate -- we don't mind if there is garbage at the end.

Question: I only read 19 bytes (which is the message) what should I truncate?

@zuckschwerdt
Copy link
Collaborator

We usually want the fields "id" and "channel". Are "homeid", "radiochannel" these? Can we rename them?

this will change all offsets

usually we add the length of the preamble to start_pos so bitbuffer_extract_bytes() puts the first data byte a 0.

only read 19 bytes (which is the message) what should I truncate?

I thought msg_bitcount is used to treat the messages as variable length. I now see it's just for with/without CRC. Ok.

Try clang-format if you can. Otherwise at least: switch( command )-> switch (command) etc.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented Apr 27, 2021

We usually want the fields "id" and "channel". Are "homeid", "radiochannel" these? Can we rename them?

Yes.
If "channel" means a radio channel normally that fits. It can bis 1-15 or 0 for all.
Id seems to be ok too, if that means a static id which is used for one "remote" in one home installation.
It seems to be a second id, which I named token, which is different per message.

So long story short, I think the rename is fine.

Try clang-format if you can. Otherwise at least: switch( command )-> switch (command) etc.

I will try. but I have no clue how to update this magic push request.
I will do my best.

@zuckschwerdt
Copy link
Collaborator

No worries then, I'll just merge manually and do the style updates and id/channel myself.

Hofyyy added 2 commits April 27, 2021 17:11
Renamed HomeID in ID and RadioChannel to channel.
The reason is a more generic naming in conjunction with other decoders in rtl_433.
used clang-format with default configuration
@Hofyyy
Copy link
Contributor Author

Hofyyy commented Apr 27, 2021

So next round. Renamed and reformatted.

Hofyyy added 2 commits April 27, 2021 18:23
- Do not read the complete raw message for the decoder anymore.
Direct jump to the real dataframe.
So all offsets defines are changed to begin at the dataframe.
That allows are better detection and "smaller" memory foodprint ...
@Hofyyy
Copy link
Contributor Author

Hofyyy commented Apr 30, 2021

Passt alles so? :-)

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 6, 2021

Ping :-)

@zuckschwerdt
Copy link
Collaborator

There are a few things to change still, I need to get around to merge and change this.

  • E.g. it should be "mic", "Integrity", DATA_COND, has_crc, DATA_STRING, "CRC",
  • Key order should be model, id, channel, ...
  • model needs to be a fixed string with no whitespace
  • if the same id can be multiple deviceType then that (model) info should be subtype, otherwise multiple model lines with DATA_COND

Can you run maintainer_update.py and commit those changes?

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 8, 2021

Thanks for the feedback. I did run maintainer_update.py

run maintainer_update.py
@zuckschwerdt
Copy link
Collaborator

Thanks. Oops, put the device last (above /* Add new decoders here. */) in include/rtl_433_devices.h and rerun please.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 8, 2021

I did the move. Sorry I cloned one decoder, and used same position to know what I have to change. So I did not see the "right" pos. But the maintainer_update.py seems to not do any further change now.

@zuckschwerdt
Copy link
Collaborator

My bad. The order comes from the compiled rtl_433, recompile then rerun the update. Sorry for the mess.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 8, 2021

No problem. Looks good now.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 8, 2021

Hello, because of the reason that you have knowledge in the 433 area. Maybe you have a hint for us in the following topic. RFD-FHEM/RFFHEM#955. With the help of your tool and the new plugin I try to do the same for the FHEM server in receive and transmit direction. But until now I was not able to receive a message from the remote. Do you know the 1011 chip? Is it easy to transfer information from a rtl422 analyse into settings for the ti1011 chip?

@merbanan
Copy link
Owner

merbanan commented May 8, 2021

Not automatically but you can use the analyzer output values as a starting point for the CC1101 chip configuration.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 8, 2021

Not automatically but you can use the analyzer output values as a starting point for the CC1101 chip configuration.
I hope the questions are not to stupid, but my knowledge starts when I receive the hex values.

  • I wrote the decoder, so I know the frequency, the sync word (0xD391D391), Deviation 20KHz (433,90 -> 433,94),

I do not know the modulation (2FSK, GFSK, MSK)
I am not sure in case of bandwidth and Datarate (Assume, 110KHz and 10 KHZ)

@merbanan
Copy link
Owner

merbanan commented May 8, 2021

@Hofyyy well if you are trying to replicate the signal then just record a sample with the signal grabber (-S) then run it through the analyzer (-A). Then compare the output with your replicated signal. When they look the same you are done. The GFSK vs 2FSK parameter can be found by looking at the signal spectra of a recorded signal. If the frequency instantly goes from the low to the high frequency it is a 2FSK signal, if it smoothly transitions it is GFSK. My bet is on GFSK.

@zuckschwerdt
Copy link
Collaborator

With MSK the data rate is tied to the deviation for narrow bandwidth, so these parameters look good. https://en.wikipedia.org/wiki/Minimum-shift_keying
Perhaps record your CC1101 signal the same way you record the original, then compare the spectrum (https://triq.org/pdv/) and actual bits decoded with -w file.ook and also pdv.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 8, 2021

Thanks for the support. I will try to learn more about it. At the moment this is like a foreign language for me. Could that be easy answered from your side with an UHR capture in complex16s? :-P

@zuckschwerdt
Copy link
Collaborator

Well, the last one is just a sum, starting one byte after the sync words (0x31 in your examples).
The one before that could be a parity check or xor-sum, but the one row with the ea0102 obviously does not fit :/

@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 12, 2021

Well, the last one is just a sum, starting one byte after the sync words (0x31 in your examples).
The one before that could be a parity check or xor-sum, but the one row with the ea0102 obviously does not fit :/

Sorry I did not get it. Could you give me one more detailed example?
08 31 22 fd 20 0a 01 0a 85
How is the first stuff a sum which gives a 85?

The EA command is something special. Maybe they calculate it also in a different way.
How would you calculate the "0a" for the first examples. Excluding the 0xEA command?

@zuckschwerdt
Copy link
Collaborator

Just add the bytes, like this BitBench.

Generate internal checksum in an dynamic. Way!
@@ -274,7 +275,7 @@ See [CONTRIBUTING.md](./docs/CONTRIBUTING.md).
[-d <RTL-SDR USB device index>] (default: 0)
[-d :<RTL-SDR USB device serial (can be set with rtl_eeprom -s)>]
To set gain for RTL-SDR use -g <gain> to set an overall gain in dB.
SoapySDR device driver is available.
SoapySDR device driver is not available.
Copy link
Owner

Choose a reason for hiding this comment

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

Stray change from the maintainer update script.

@@ -151,7 +151,7 @@ RTL\-SDR device driver is available.
To set gain for RTL\-SDR use \-g <gain> to set an overall gain in dB.
.RE
.RS
SoapySDR device driver is available.
SoapySDR device driver is not available.
Copy link
Owner

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to be that the *.1 is a local generated file?
And its coming from #ifdef SOAPYSDR
"\tSoapySDR device driver is available.\n"
#else
"\tSoapySDR device driver is not available.\n"
#endif

I can correct the Readme.md or what should I do?

src/devices/rojaflex.c Outdated Show resolved Hide resolved
src/devices/rojaflex.c Outdated Show resolved Hide resolved
src/devices/rojaflex.c Outdated Show resolved Hide resolved
Hofyyy added 3 commits May 13, 2021 17:39
- Added pramble desciption
- Added syn word desciption
- Added token part II desciption
- Added fprintf guards
Changed CRC output to Data_COND
@Hofyyy
Copy link
Contributor Author

Hofyyy commented May 18, 2021

@merbanan Could you bring this through the door? I will not be able to provide much during this week.

@zuckschwerdt zuckschwerdt self-assigned this May 18, 2021
@zuckschwerdt
Copy link
Collaborator

I'll be merging this, there are some final touches needed. Maybe on the weekend.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented Jun 6, 2021

Sorry for pushing, but the RTL software is great and I would like to have official Rojaflex support :-)

@zuckschwerdt
Copy link
Collaborator

My bad. I'll get on it.

@zuckschwerdt
Copy link
Collaborator

Merged, but note that there are some changes in the output to better represent the fields.

@zuckschwerdt
Copy link
Collaborator

@merbanan I've updated the code style and merged this, but I feel the model should be fixed to the protocol as "RojaFlex-Remote" with an additonal key of the decoded "device" as "Remote"/"Bridge"/"Shutter"/"Unknown". What do you think?

@Hofyyy
Copy link
Contributor Author

Hofyyy commented Jun 7, 2021

Only for information, at the moment it can decode messages from the remote and from the shutter. and you can detect that from the message type. I do not know if that fits to your question.

@zuckschwerdt
Copy link
Collaborator

Yes, we do want a key to show "Remote"/"Bridge"/"Shutter"/"Unknown". But best case that should not be the "model" key, we use that more like a protocol-type information.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented Jun 7, 2021

Ok, I do not know the original Protocol specification, so I only can assume it.
It seems to be that the only key which shows it is the following:

e.g.
if ((msg[COMMAND_ID_OFFSET] & 0xF) == 0x5) {
deviceType = "RojaFlex-Shutter";
}

@zuckschwerdt
Copy link
Collaborator

With "keys" I'm referring to our common set of output fields, s.a. https://triq.org/rtl_433/DATA_FORMAT.html
We currently use e.g. "model", "RojaFlex-Shutter" but it should rather be

"model", "RojaFlex"
"subtype", "Shutter"

that's my thought anyway ;)

@Hofyyy
Copy link
Contributor Author

Hofyyy commented Jun 7, 2021

Its fine for me :)

@zuckschwerdt
Copy link
Collaborator

Great! The convention for "model" values is "Xyz-Abc". We usually call advanced "remote control"-type protocols "Xy-Security". Some like the Nexa/Proove are generic "command to group-unit", so this would fit. Model values are not meant to convey a specific use-case anyway. But we could perhaps call this "RojaFlex-Automation", maybe. Not sure.

@Hofyyy
Copy link
Contributor Author

Hofyyy commented Jun 7, 2021

I am not sure how to implement this in a quick way. I will check it next weekend. If you would like to change it, because you know how to do it. Feel free.

@zuckschwerdt
Copy link
Collaborator

No worries, I'll change that as needed. Just waiting for @merbanan to help out with:

  • split in "model" and "subtype"?
  • model of "RojaFlex-Security" (as usual) or "RojaFlex-Automation" or maybe just "RojaFlex"?

@merbanan
Copy link
Owner

merbanan commented Jun 7, 2021

The main aim in the output was always to make it descriptive and as unique as possible. Adding subtype makes sense. There might be other cases where this can be useful.

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.

3 participants