search

srobo / competition-simulator

A simulator for Student Robotics Virtual Competitions
https://studentrobotics.org/docs/simulator/
MIT License
8 stars 2 forks source link

Update arduino simulation for SR2024 #412

Closed PeterJCLaw closed 9 months ago

PeterJCLaw commented 10 months ago

This is sort-of a copy over of the Pin class from the kits, thus picking up various input & mode checks, with modifications to work in the context of the simulator.

Fixes https://github.com/srobo/competition-simulator/issues/411 Fixes https://github.com/srobo/competition-simulator/issues/380

Review by commit may be useful -- there's lots of renaming (only) in the first commit, then functional changes & refactoring in the second.

Tested by manually using the keyboard robot.

WillB97 commented 10 months ago

The codebase seems to differ significantly from sr-robot. Can we not maintain the structure and replace the points where the SerialWrapper is passed in with a DeviceWrapper that handles what each device type does for the read and writes?

PeterJCLaw commented 10 months ago

The codebase seems to differ significantly from sr-robot. Can we not maintain the structure and replace the points where the SerialWrapper is passed in with a DeviceWrapper that handles what each device type does for the read and writes?

This would imply implementing a fully generic serial interface right? That feels like it would be a lot more complicated and create coupling not only to the public API but also the internals of the kit code.

WillB97 commented 10 months ago

What I'm suggesting is having a single Pin class which a device wrapper is then passed to the constructor (similar to how the SerialWrapper is passed to the pin in sr-robot). Each wrapper contains functions to handle what is done on the webots device for analog read, digital write & digital read. The pin class then just calls these functions on its wrapper object.

PeterJCLaw commented 10 months ago

What I'm suggesting is having a single Pin class which a device wrapper is then passed to the constructor (similar to how the SerialWrapper is passed to the pin in sr-robot). Each wrapper contains functions to handle what is done on the webots device for analog read, digital write & digital read. The pin class then just calls these functions on its wrapper object.

So I had a play with this, in part to see if we could avoid the __slots__ thing and also whether the microswitch class having an analogue read method makes sense.

It ends up being a bit complicated if we want the type system to be at all useful for us. Obviously we could force all devices to implement all three functions (analogue read, digital read & write), however that then ends up with weirdness like microswitches needing to implement analogue read. It also means that checking the "supports analogue" stuff against what's actually on the pin is harder. (This does match reality, however probably isn't a property we want given that this is a robot we're building that we want to be reliable rather than something competitors can change).

I'm going to go with the current approach for now (including dropping __slots__) so that we have something which works. We can worry about refactoring it later.

PeterJCLaw commented 10 months ago

https://github.com/srobo/competition-simulator/pull/412/commits/cc1be6adea08a2e3669fed0067c7ef8fee9dff09 ends up separating out pins from devices.

PeterJCLaw commented 9 months ago

CI failures here are due to mypy upgrade which has been fixed on main.