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

Add GPS receiver device to provide senseloc capability #956

Merged
merged 6 commits into from
Jan 5, 2023
Merged

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented Jan 4, 2023

Add a new GPS receiver device which enables the senseloc capability (and hence the whereami command). Towards #26 .

  • My immediate motivation is that I want to be able to define excursion : cmd unit -> cmd unit which executes the given command and then returns to the same location and orientation as before. Being able to use the whereami command is one of the last missing pieces of machinery necessary to be able to do this in classic mode (the other is a heading command, see New heading command #955).
  • The proposed recipe for GPS receiver is antenna + circuit + clock + compass, which is a somewhat difficult recipe (antenna requires silver which requires a deep mine; a clock requires quartz + a bunch of iron gears, etc.). One might wonder whether we should make the recipe easier since finding out where you are seems like a kind of fundamental operation. However, consider that in order to even be able to make use of the result of whereami you need at least (1) an ADT calculator to deal with the pair (which transitively requires typewriter -> circuit -> silicon -> quartz) (2) probably things like comparator and calculator to do anything useful with the coordinates. By the time you have those things you can definitely already build circuit + clock + compass, and you're probably not that far away from getting some silver for an antenna. Also, in practice it's an interesting/fun constraint to build up machinery that has to work entirely based on relative position without being able to find out your absolute coordinates.
  • For some reason this is causing Testing/508-capability-subset to fail. I think perhaps it is due to Wrong device reported missing #397 ? I will investigate. EDIT: unfortunately, that wasn't it!

@xsebek
Copy link
Member

xsebek commented Jan 4, 2023

What is going on? 😄

Testing/508-capability-subset:                                                                       FAIL (0.28s)
  test/integration/Main.hs:263:
        Player exception: Fatal error: BASE: I could not build my robot! What happened?!
  Use -p '/Regression tests.Testing\/508-capability-subset/' to rerun this test only.

@byorgey
Copy link
Member Author

byorgey commented Jan 4, 2023

@xsebek In short, I think capability analysis is broken when there are multiple devices that provide multiple same capabilities. I've been working on #397 (which I think is the culprit) but don't quite have it all ironed out yet. See my comments on that issue. I'll see if I can finish that up next and hopefully it will resolve this as well.

@byorgey
Copy link
Member Author

byorgey commented Jan 5, 2023

Unfortunately fixing #397 didn't resolve the failing test. The build is failing with an error saying "You do not have required devices, please obtain: GPS receiver", but I don't understand why. The GPS and Tardis entities defined in the scenario both provide the senseloc capability so it shouldn't matter that we don't have a GPS receiver. But it's like it's not seeing those.

@byorgey
Copy link
Member Author

byorgey commented Jan 5, 2023

doh!

instance Semigroup EntityMap where
EntityMap n1 c1 <> EntityMap n2 c2 = EntityMap (n1 <> n2) (c1 <> c2)

Hint: combining two Map Capability [Entity] values with <> doesn't exactly do what we want. 🤦

@byorgey byorgey requested review from xsebek and kostmo January 5, 2023 21:27
@byorgey
Copy link
Member Author

byorgey commented Jan 5, 2023

Well, finally got the tests to pass. Even though #397 wasn't the cause I'm glad I thought it was, because it gave me the push to finally get that sorted. 😁

description: |
Test the `whereami` command and GPS receiver device.
https://github.com/swarm-game/swarm/issues/956
objectives:
Copy link
Member

Choose a reason for hiding this comment

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

Add a goal here that explains what the solution is doing to meet the condition

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, good idea. While I was at it I also thought of a way to make the test much better --- what do you think now?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, using random is great.

@byorgey byorgey requested a review from kostmo January 5, 2023 22:34
@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Jan 5, 2023
@mergify mergify bot merged commit 46d7c96 into main Jan 5, 2023
@mergify mergify bot deleted the feature/GPS branch January 5, 2023 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants