fix node name bug, when it attributes a namespace (and also fix #142)

[pojiro]

This commit fixes below, so closes #142

>>> [rcutils|error_handling.c:108] rcutils_set_error_state()
This error state is being overwritten:

  'node name must not contain characters other than alphanumerics or '_', at /tmp/binarydeb/ros-foxy-rcl-1.1.13/src/rcl/node.c:164'

with this new error message:

  'subscription's implementation is invalid, at /tmp/binarydeb/ros-foxy-rcl-1.1.13/src/rcl/subscription.c:462'

rcutils_reset_error() should be called after error handling to avoid this.
<<<

本修正方法だと、ノード名に namespace を含まなくなります。
それが rclex の方針と異なるようであれば、修正案をご指示いただきたいです。
※ただし、rclのリソースとしてはnamespeceを保持する点は維持しています。

修正前： "#{namespace}/#{node_id}"
修正後: "#{node_id}"

[takasehideki]

代案はありそうなのでちょっと調査時間をください．今日中には結論出します．

[takasehideki]

あれっ,,, もしかして _with_namespace といってるけどちゃんと実装されていないということ？？

https://github.com/rclex/rclex/blob/main/lib/rclex/resource_server.ex#L30-L38

https://github.com/rclex/rclex/blob/main/lib/rclex/resource_server.ex#L65-L69

@s-hosoai これがYesであれば，このようにしたいです．

• このPRはこのままmerge
• _with_namespace をちゃんと実装する的なIssueを立てる

[s-hosoai]

@takasehideki @pojiro
コード確認してきました。すみません、私の実装ミスでした。
resource_server.ex:198,211あたりでnode_identifierとしてns+node_name+IDとしちゃってますが、ここはnode_nameで回すべきですね。別Issueにして貰えればこちらで修正します。

[s-hosoai]

本PR、まだ動作確認していないため、確認＋レビュー後にマージしてしまいますね。

[takasehideki]

ん？念のため確認ですが，

resource_server.ex:198,211あたりでnode_identifierとしてns+node_name+IDとしちゃってますが、ここはnode_nameで回すべきですね。

これは本PRで直してもらっているのでは？
https://github.com/rclex/rclex/pull/149/files#diff-a5818b6cb433a1fbc21187b43abed14c40d0cd4441609535ba056beb2fda11b9

[s-hosoai]

あ、、ほんまですね・・。ちゃんと現状把握してからコメントしないとだめですね。。少々お待ちを。

[s-hosoai]

かんぜんにはあくしました。
201行目の同名ノードはじきの所の挙動をどうするかってところがネックですね。name_space違いの同名node_nameを許容するか否かですが、個人的には折角name_spaceで分けているのだから別のものとしたいです。（@takasehideki 判断お願いします）
node_identifierとして使っているのはここと返り値ですが、返り値も呼び出し元でnamespaceは分かっているので、node_name+IDのリストを返すだけでよさそうですし、201の中でだけns+nameとして判断するようにすればよいかと。
ついでに、変数名identifierは外してもいいかもです。（ns+nameでシステム内の一意な名前の意図でつけてました。）

[takasehideki]

とりあえず本質的な問題が洗い出せたと認識しています．

@s-hosoai まずはこのPRの扱いをどうしましょうか？

1. これはPR名通りの対応がされたものとしてmergeする（conflict解消も必要ですが,,,
2. 本質的な問題の解決にはなっていないので，closeして新しくPRで対応する
3. PR名を変更し，このまま続ける（"fix node name bug, when it attributes a namespace" など）

@pojiro いずれにせよ本件はこちらで引き取ります．問題を明らかにしていただきありがとうございます！！

[takasehideki]

とりあえずここで回答を続けますが，

201行目の同名ノードはじきの所の挙動をどうするかってところがネックですね。name_space違いの同名node_nameを許容するか否かですが、個人的には折角name_spaceで分けているのだから別のものとしたいです。（@takasehideki 判断お願いします）

はい，別のものとすべきです．

node_identifierとして使っているのはここと返り値ですが、返り値も呼び出し元でnamespaceは分かっているので、node_name+IDのリストを返すだけでよさそうですし、201の中でだけns+nameとして判断するようにすればよいかと。
ついでに、変数名identifierは外してもいいかもです。（ns+nameでシステム内の一意な名前の意図でつけてました。）

node_identifier は複数ノード生成の create_nodes 等で用いられるものかと．引数で与えられた node_name に prefix を付けて生成するやつですね．本質的には create_node の場合は prefix を付けないなど対応が必要かと思います．

[takasehideki]

あとROS 2におけるnamespaceの関係資料です．topic,serviceに関するものですが，ノードについても同じ概念で名前空間を分けています．
https://design.ros2.org/articles/topic_and_service_names.html#namespaces

[s-hosoai]

@takasehideki node_nameの重複確認は本件の範疇内だと思うので、このPR内で修正してしまう方が妥当な気がします。
よく見たら、今までの作成済みのnamespace+node_nameはstateとして持たせなきゃ比較できないっすね。となるとやっぱり返り値で返さにゃならんか・・。とりあえずこちらで修正してみます。

[s-hosoai]

@takasehideki @pojiro
MapのキーやGenServerのIDにするのに一意なキーが必要なので、結局Rclexとして保持するIDは#{namespace}/#{node_name}に戻しました。今のところ、create_nodesで作ったIDのリストを持ちまわして使っているので特に不便はないですが、追々finishやstopにnameとnamespaceを引数に取るAPIも追加した方がいいかもしれません。

[takasehideki]

これだと #142 が再燃しませんか？

[s-hosoai]

あ、NIF側ではしっかり分けてるのでissueのやつには引っかからないです。NIF側とrclex側のIDの扱いがごっちゃになってたのが要因なので、そのあたりは解決できたと思います。

[takasehideki]

それは失礼しました,,, あとでちゃんと動作確認してからmergeします！

[takasehideki]

問題ないことを確認しました．mergeします！