ros-navigation / navigation2_dynamic

Navigation2's dynamic obstacle detection, tracking, and processing pipelines.
Apache License 2.0
44 stars 14 forks source link

uuid for obstacle message #23

Closed simutisernestas closed 3 years ago

simutisernestas commented 3 years ago

don't really like the solution of the tracking of those Rviz markers ids like that, but can't think of a better way to do it for now

simutisernestas commented 3 years ago

i haven't tested it yet on real data, i'll ping you after I do that

SteveMacenski commented 3 years ago

Those changes make 3 options to ~11 options. Definitely an improvement, but not really randomly distributed.

I did some playing uuid.uuid1().int % 100 will give you 25 evenly distributed numbers between 0 and 100 (so divide by 100). From 1000 iterations:

{81: 40,
 17: 40,
 53: 40,
 89: 40,
 25: 40,
 61: 40,
 97: 40,
 33: 40,
 69: 40,
 5: 40,
 41: 40,
 77: 40,
 13: 40,
 49: 40,
 85: 40,
 21: 40,
 57: 40,
 93: 40,
 29: 40,
 65: 40,
 1: 40,
 37: 40,
 73: 40,
 9: 40,
 45: 40}

Each of those values appears exactly 40 times. Still not random, but closer.

simutisernestas commented 3 years ago

uuid.uuid4().int % 100 gives all 100 distributed like this:

31: 12
57: 13
18: 11
1: 9
98: 7
96: 5
6: 18
44: 13
81: 9
3: 9
10: 13
11: 11
93: 12
68: 13
55: 8
91: 13
67: 14
26: 15
41: 11
8: 12
94: 8
61: 16
25: 7
99: 14
43: 9
37: 10
23: 17
73: 15
63: 9
85: 9
20: 7
75: 11
35: 15
33: 8
84: 13
58: 11
74: 7
32: 4
72: 5
70: 9
59: 10
80: 14
64: 7
83: 11
34: 10
60: 13
21: 8
90: 7
28: 11
65: 7
54: 8
50: 12
92: 10
4: 11
56: 11
53: 6
86: 11
89: 8
16: 18
78: 7
87: 12
48: 8
97: 10
95: 6
39: 9
27: 7
69: 11
66: 8
51: 11
79: 4
71: 9
49: 7
47: 8
9: 12
46: 8
42: 10
29: 12
62: 12
12: 12
45: 8
88: 15
13: 5
30: 15
15: 12
77: 7
22: 15
14: 10
38: 10
76: 11
2: 5
52: 5
82: 5
7: 9
40: 7
19: 12
36: 9
0: 8
24: 7
17: 11
5: 6

Looks much better. I've thought that uuid1 will be more random because of the time component. Should I use this?

SteveMacenski commented 3 years ago

Use uuid4, googling around makes me think this is a good tradeoff.

simutisernestas commented 3 years ago

I've found some major bugs while testing, so there will be quite some changes

simutisernestas commented 3 years ago

Now it's tested and working properly. The first implementation was too naive. Assigning python UUID object directly to ROS msg was not a good idea :). Script below should explain those changes better:

import uuid 
from unique_identifier_msgs.msg import UUID

if __name__ == '__main__':
    raw_uuid = uuid.uuid4()
    print(raw_uuid)

    uuid_msg = UUID()
    print(raw_uuid.bytes)

    uuid_msg.uuid = list(raw_uuid.bytes)
    print(uuid_msg.uuid)
    print(bytes(uuid_msg.uuid))

    a = uuid.UUID(bytes=bytes(uuid_msg.uuid))
    print(a)

Output:


b9db0cd7-68a4-4079-9228-2b8991f57def
b'\xb9\xdb\x0c\xd7h\xa4@y\x92(+\x89\x91\xf5}\xef'
[185 219  12 215 104 164  64 121 146  40  43 137 145 245 125 239]
b'\xb9\xdb\x0c\xd7h\xa4@y\x92(+\x89\x91\xf5}\xef'
b9db0cd7-68a4-4079-9228-2b8991f57def
tony23545 commented 3 years ago

It looks good to me.

SteveMacenski commented 3 years ago

@simutisernestas any other changes before I can merge?

simutisernestas commented 3 years ago

GTG already