Update to platform to allow it to control the positions of its sensors #177
Conversation
|
One thing we could do to simplify things, is require that a sensor be mounted to a platform, and can't operate alone; and make the base Platform have sensors (having the option to set this to empty list). The base platform could be stationary, and then have a subclass for a moving platform. The position and velocity information should using mappings, such you have greater flexibility in models/dimensions that you use. I think then you can also have position and velocity as an attribute that's dynamically returned (as appose to "get" methods) from the platform state using the position/velocity mappings on the platforms, and using the mappings and offsets on the sensor. Velocity can still be present on the base (stationary) platform, just returning StateVector of zeros in the same dimensions as position mapping. On mounting offsets, we should just make this a list of StateVector, that must be same length as number of sensors (default could still create list of all zero state vectors). The current description and logic is confusing. Also, I'm not clear on the role of add_sensor? Are we expecting to add (remove?) sensors during simulation? Could just be covered by adding a sensor and offset to the platform if required? |
Intriguing. I don't see any reason it wouldn't work and I'd be happy to implement this if that is the consensus.
Agreed. I wondered about that, but it wasn't the approach used in the
I think that should work.
Do you mean mounting mappings? Mounting offsets are implemented as you describe, but mappings are more complex. Assuming you do mean mappings, then yes, I can see the argument for simplifying this code. The tradeoff is more complicated client code slightly though, as I don't think zero defaults would work. The only sensible default I can see for mappings is Re-reading the comment, it also possible that you meant we should use
I have to say I added this because there was a TODO asking for it and it was easy when I was refactoring. The Without the |
|
On reflection: I think that |
…s, rather than saving it to the sensor each time the platform moves.
- Create both fixed and moving sensor platforms - Force a sensor to be mounted to a platform - properties for position and orientation - Use a mapping between state_vector and position - Give Platform a state_vector property - Force mappings to be sensible shape - Update tests to match Still needs some tidying and documentation
… all Sensors now require platforms. Passive test failing, but will be addressed in the next commit.
erogers-dstl
force-pushed
the
platform_extensions
branch
from
7d6bc7b
to
f5401ae
Compare
|
Still a WIP. Mostly needs documentation. Pushed to check the circle CI builds. I expected the docs not to build until #183 is merged. |
…he platform property, rather than platform system.
# Conflicts: # stonesoup/platform/base.py # stonesoup/platform/simple.py # stonesoup/sensor/radar/radar.py
…bly, without excluding any use cases.
… model is not set.
erogers-dstl
force-pushed
the
platform_extensions
branch
from
15f57d3
to
e7995b5
Compare
stonesoup/sensor/base.py
Outdated
| 'default platform') | ||
|
|
||
| @property | ||
| def orientation(self): |
There was a problem hiding this comment.
| def orientation(self): | |
| def orientation(self) -> StateVector: |
|
Just adding a few more tests to improve coverage... |
All done! |
Update to platform to allow it to control the positions of its sensors rather than saving it to the sensor each time the platform moves.
As discussed with Rich Green (what seems like an age ago).
Also an add_sensor method and tests (as requested by the todo in the code).
Changes needed to Radar tests for the new platforms.