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

PacketSerial Needs Mocking For Testing Library #57

Open
X-sam opened this issue Feb 27, 2023 · 3 comments · May be fixed by #162
Open

PacketSerial Needs Mocking For Testing Library #57

X-sam opened this issue Feb 27, 2023 · 3 comments · May be fixed by #162
Assignees
Milestone

Comments

@X-sam
Copy link
Member

X-sam commented Feb 27, 2023

#56 disabled tests that expected the old serial library, but we need to migrate PacketSerial to using our mocking strategy.

@t0mpr1c3
Copy link
Contributor

t0mpr1c3 commented May 2, 2023

I'll have a stab at it.

@t0mpr1c3 t0mpr1c3 self-assigned this May 2, 2023
@t0mpr1c3 t0mpr1c3 added this to the v1.0 milestone May 2, 2023
@t0mpr1c3
Copy link
Contributor

t0mpr1c3 commented May 18, 2023

A problem: PacketSerial depends on Stream, which depends on Print, which depends on WString, which happens to define a macro called F. This causes a clash with arduino-mock/Arduino.h which defines a different macro F.

This is the relevant line in the AYAB fork of arduino-mock:
https://github.com/AllYarnsAreBeautiful/arduino-mock/blob/e59117bd75d4e3f05f86249f70e8cea356b527ef/include/arduino-mock/Arduino.h#L77C11-L77C11

It might be possible to #undef F somewhere and fix this, but I am tempted to kick this can down the road and live with the 99% coverage.

@t0mpr1c3 t0mpr1c3 removed their assignment May 18, 2023
@t0mpr1c3 t0mpr1c3 moved this from Todo to In Progress in AYAB 1.0.0 Release Tracking May 19, 2023
@t0mpr1c3 t0mpr1c3 modified the milestones: v1.0, next Jun 3, 2023
This was referenced Oct 1, 2023
@t0mpr1c3 t0mpr1c3 linked a pull request Oct 1, 2023 that will close this issue
@t0mpr1c3
Copy link
Contributor

I have a fix for this, but it's on top of a lot of other changes which are not necessary for v1.0, so I'm going to suggest postponing this issue.

@t0mpr1c3 t0mpr1c3 self-assigned this Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants