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

enrich README #24

Merged
merged 13 commits into from
Feb 22, 2024
Merged

enrich README #24

merged 13 commits into from
Feb 22, 2024

Conversation

takasehideki
Copy link
Contributor

READMEをかき揚げてみました!
変なことを書いてないか確認してもらいたいです.ツッコミ大歓迎です! > @pojiro @s-hosoai


Zenohex is the [zenoh](https://zenoh.io/) client library for elixir.
Zenohex is Elixir API for [Zenoh](https://zenoh.io/).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

zenoh-pythonなど他のライブラリもこのような書き方(e.g. Python API for Zenoh)なので合わせました.

README.md Outdated
Comment on lines 10 to 26
Zenoh is a new protocol for Zero Overhead Pub/Sub, Store/Query and Compute.
The most obvious explanation is that Zenoh offers publication subscription-based communication capabilities.
Within the same network, Zenoh can autonomously search for communication partner nodes like DDS.
Between different networks, Zenoh can search for nodes through a broker (called a router in Zenoh) like MQTT.
Also, Zenoh provides functions for database operations and computational processing based on the Key-Value Store.
Moreover, it has plugins/bridges for interoperability with MQTT, DDS, REST, etc. as communication middleware, and influxdb, RocksDB, etc. as database stacks.

## Installation
For more details about Zenoh, please refer to the official resources.
- [Official Page](https://zenoh.io/)
- [GitHub](https://github.com/eclipse-zenoh/zenoh)
- [Discord](https://discord.gg/vSDSpqnbkm)

If [available in Hex](https://hex.pm/docs/publish), the package can be installed
by adding `zenohex` to your list of dependencies in `mix.exs`:
Zenoh's core modules are implemented in Rust, but API libraries in various programming languages such as Python ([zenoh-python](https://github.com/eclipse-zenoh/zenoh-python)), C ([zenoh-c](https://github.com/eclipse-zenoh/zenoh-c)), C++ ([zenoh-cpp](https://github.com/eclipse-zenoh/zenoh-cpp)) are officially provided.

```elixir
def deps do
[
{:zenohex, "~> 0.2.0-rc.2"}
]
end
```
So what we need is [Elixir](https://elixir-lang.org/)!
With this library, you can call Zenoh from your Elixir application to perform its basic processing.
This allows the creation and communication of a large number of fault-tolerant nodes with little memory load (we hope :D
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ちょっと前置きとしては長いんですが,,, 初めてこれを見る alchemist に Zenoh への興味を惹いてもらうために,,,:D

README.md Outdated Show resolved Hide resolved
## Getting Started
This repository uses [Rustler](https://github.com/rusterlium/rustler) to call Rust (Zenoh) modules from Elixir, and pre-compiled NIF modules are automatically downloaded at `mix compile` time (since v0.1.3).
IOW, if you just want to use this library from your Elixir application, you do not need to prepare a Rust environment.
If you still want to build Rust NIF modules locally, please refer to [this section](#build-nif-module-locally).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

元のだと [optional] を見逃すと Rust が必要そうに読めなくもなかったので,ちょっと明確化しました.


Zenohex has a policy of providing APIs that wrap the basic functionality of Zenoh like other API libraries.

Here is the first step to building an Elixir application and using this feature.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

このライブラリの方針を明確化しつつ,これは簡単な例だよってことにしました.

{:ok, #Reference<>}
iex(3)> {:ok, subscriber} = Zenohex.Session.declare_subscriber(session, "pub/sub")
iex()> {:ok, subscriber} = Zenohex.Session.declare_subscriber(session, "demo/**")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

key名はいちばん使ってそうな Examples 例に合わせることにしました
https://github.com/eclipse-zenoh/zenoh-python/tree/master/examples#z_pub

Comment on lines -59 to +86
{:ok, "Hello Zenoh Dragon"}
iex(6)> Zenohex.Subscriber.recv_timeout(subscriber, 1000)
iex()> Zenohex.Subscriber.recv_timeout(subscriber, 1000)
{:ok,
%Zenohex.Sample{
key_expr: "demo/example/test",
value: "Hello Zenoh Dragon",
kind: :put,
reference: #Reference<>
}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

これ私の環境ではこのように出力されましたが,意図として合っていたでしょうか?

Copy link
Contributor

Choose a reason for hiding this comment

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

あってます!途中で データそのまま(integer, float, binary)を返すだけでは設計が成り立たないことが分かって、Sample 構造体を返すようにしました。 私の範囲での README.md の修正し忘れです 🙇


For most users, this section should be skipped.

### Build NIF module locally
Copy link
Contributor Author

Choose a reason for hiding this comment

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

自分でNIFコンパイルしたい方向けの情報はこのセクションにまとめました

end
```

### Enhance mix test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mix testに関するTIPS的なところをここにまとめました.
いちばん自信がないので特に確認いただきたいです! > @pojiro

Copy link
Contributor

Choose a reason for hiding this comment

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

ばっちり、問題なしです。


### How to release

These steps just follow the [Recommended flow of rustler_precompiled](https://hexdocs.pm/rustler_precompiled/precompilation_guide.html#recommended-flow).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

このセクションのバージョン番号は都度で更新したくないので,もう v0.2.0 ってことにしました.

Copy link
Contributor Author

@takasehideki takasehideki left a comment

Choose a reason for hiding this comment

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

いろいろ補足を入れました!

Copy link
Contributor

@pojiro pojiro left a comment

Choose a reason for hiding this comment

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

内容はすばらしくて特に指摘事項はありません。

それと別で見つけた些末な修正提案を出しています。

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
takasehideki and others added 4 commits February 22, 2024 10:01
Co-authored-by: Ryota Kinukawa <[email protected]>
Co-authored-by: Ryota Kinukawa <[email protected]>
Copy link
Contributor

@pojiro pojiro left a comment

Choose a reason for hiding this comment

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

zenohex_on_nerves の中身を作ることを約束。

README.md Outdated Show resolved Hide resolved
@pojiro pojiro self-requested a review February 22, 2024 01:28
USE_DIFFERENT_SESSION="1" mix test
```

FYI, CI does this in `test-with-another-session` without defining `SCOUTING_DELAY`, so this test sometimes fails on GHA.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pojiro CIが実際に落ちることがあるので aaad576 にて注記しました(自分が忘れないために:D
https://github.com/b5g-ex/zenohex/actions/runs/7997846597/job/21843046901

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"without defining SCOUTING_DELAY" だと思っていましたが,合っていたでしょうか?

Copy link
Contributor

Choose a reason for hiding this comment

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

USE_DIFFERENT_SESSION="1" の場合、SCOUTING_DELAY が短いより長い方が落ちにくくなるのは事実。
だけど、長くても(デフォルトの200ms としても)落ちる時があるというのが私の感触です。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます!説明文としては間違っていない(SCOUTING_DELAYは定義していない)と認識しました.
ここで言いたかったことは test-with-another-session だけ落ちることあるけどトランキーロだぜっという自分たち向けのメモなので,不足ないということにします!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

なにかが足りてない,,, と思ったら SCOUTING_DELAY のデフォルト値が抜けてました.ので 78ac00b にて追加しました!

Copy link
Collaborator

@s-hosoai s-hosoai left a comment

Choose a reason for hiding this comment

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

ありがとうございます!変更箇所と全体を見直しましたが,とてもええ感じです!

@takasehideki takasehideki merged commit cd6678b into main Feb 22, 2024
4 checks passed
@takasehideki takasehideki deleted the readme branch February 22, 2024 04:39
USE_DIFFERENT_SESSION="1" mix test
```

FYI, CI does this in `test-with-another-session` without defining `SCOUTING_DELAY`, so this test sometimes fails on GHA.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FYI, CI does this in `test-with-another-session` without defining `SCOUTING_DELAY`, so this test sometimes fails on GHA.
FYI, CI does this in `test-with-another-session` without defining `SCOUTING_DELAY`.
This test may fails on GHA, it depends on whether the scouting is successful or not.

落ちることもあるというニュアンスを残す文章にするならこうはどうでしょう?

Copy link
Contributor

Choose a reason for hiding this comment

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

一足遅かった笑、🆗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ごめんなさい!これ merge しちゃったのでこっちに移動させます!(もうひとつ修正漏れもあったので,,,
#25

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