Review FIXME comments

[Poikilos]

More in FIXME and maybe more not? Tests pass, but try examples again. I'm not completely sure how to interpret the output (to determine whether it is as expected). At least one FIXME comment is added in #32, but the comments are regarding code that predates the PR.

[bobjacobsen]

#37 fixes several of these. Not sure what to do about two of the remaining ones:

• ./tests/test_physicallayer.py:10: # FIXME: finish this

PhysicalLayer only contains three pass methods. It's there because of the migration from Swift. Perhaps the thing to do is to remove the test file entirely?

• ./openlcb/memoryservice.py:301: # FIXME: Python resizes ints automatically based on value.

Doesn't seem to be a problem there. What am I missing?

[Poikilos]

• ./tests/test_physicallayer.py:10: # FIXME: finish this

PhysicalLayer only contains three pass methods. It's there because of the migration from Swift. Perhaps the thing to do is to remove the test file entirely?

Maybe keep it as a placeholder so we can easily add a test in case we figure out a way to test parts of the submodule without hardware attached. If it doesn't seem necessary now, maybe just change FIXME: finish this to TODO: implement offline tests if relevant in cases where tests only have "pass"

[Poikilos]

• ./openlcb/memoryservice.py:301: # FIXME: Python resizes ints automatically based on value.

Doesn't seem to be a problem there. What am I missing?

If it doesn't cause a problem, maybe just change:

        Returns:
            int: The converted data as a number.
        """
        # FIXME: Python resizes ints automatically based on value.

to a clarification (since return size may contradict method name arrayToUInt64, but not cause a problem):

        Returns:
            int: The converted data as a number (Python determines
                actual in-memory size based on the value).
        """

[bobjacobsen]

Thanks for showing how you wanted those fixed. Added to #37.

[Poikilos]

I searched for "FIXME" again and I think everything is covered if you make that change to the comments. Ready to merge #37 after that?

[Poikilos]

I went ahead and made the changes we discussed here, so to get those you can review and pull #39 which merges into the fixme_once branch, before merging #37 if you see #39 is acceptable.

[bobjacobsen]

Oops. I merged #37 before merging your #39, which closed #39. Will fix by created a new PR that merges your branch to main. Sorry for my confusion.