Skip to content

Conversation

@Poikilos
Copy link
Contributor

@Poikilos Poikilos commented May 15, 2025

Hello, I hope this finds you well. Most changes suggested by Balazs (#62 (comment)) are here.

  • State machine (more CanLink states)
  • Error or collision reverses the reservation process
    • All queued frames related to a previous reservation are cleared when defineAndReserveAlias is called.
    • In case the application already grabbed a bad frame (via pollFrame), that race condition can be eliminated by using isBadReservation(frame) isCanceled (checks if the frame is part of a bad reservation or not in the case of CanLink implementation).
  • non-blocking port access prevents overlapping calls to ports. Allows two-way communication during alias reservation (taking turns) and eliminates:
    • missing responses during alias reservation (using an event loop and queue, and calling sendAll and receiveAll separately. This allows single-threaded or multi-threaded applications to work equally well, and makes both less brittle and more intuitive to use with respect to timing).
    • undefined behavior caused by overlapping port read/write
  • (above is facilitated by) network layer order is modeled consistently (socket <-> [PhysicalLayer <-> other layers] <-> application) for stability and clarity. For example (for both reasons), no unexpected (difficult to predict or trace) port use in deep listener callstacks.
  • PortInterface superclass added to TcpSocket and SerialLink allows the openlcb stack (or other code) to utilize either type of port without conditional code or subclassing OpenLCB protocol implementation classes.
  • Task list: processCollision calls defineAndReserveAlias but can't cancel previously-called (concurrent) call #62

All tests pass locally, so I'm not sure why GitHub isn't noticing that. Maybe it will catch up and go green. We'll see. All tests pass, and:

  • Use unittest auto-discovery in github workflow (run-tests.yml): Now it runs all available test files regardless of files being added, renamed, or removed (in this case--there wasn't an abstract LinkLayer test. The file was blank).

  • There are some things that may be simplified, but I thought it would be good if we both can review this at the same time to get back in sync. Make high-level methods (sendAll and receiveAll) to simplify usage related to the thread-safe refactor.

Poikilos and others added 30 commits February 5, 2025 09:59
…t correctly in flush (Fix a breakage from my utf8 PR). Enforce str all the way. Improve comments.
…in endElement). Add to sys.path only if can be done accurately.
… level manually set to logging.INFO or more verbose). Switch create a logger (instead of using logging directly) to make the source of messages clear, that being canlink.py in this case. Add & improve docstrings.
… nodeIdToAlias to populate: Fix bobjacobsen#62). Stream XML continuously (use parser.feed) so CDI branches trigger callbacks as they download.
…s start of next). Comment questionable check until question in issue bobjacobsen#63 is answered.
…rrent reservation process (collision handler runs it again) unless no collision (fix bobjacobsen#62) as per section 6.2.1 of CAN Frame Tansfer Standard. Fix type for CanLink.State values (some were tuples due to trailing comma, now all are ints--still would compare if the value was always set and compared from the Statet(Enum) subclass, but not technically correct) (related to issue bobjacobsen#62). Add a related docstring.
…Permitted before sending messages (related to issue bobjacobsen#62). Use only one receive thread in cdihandler to avoid missing packets (and rename CDIHandler to PortHandler so as to allow handling different types of messages and memos in future versions). Rename "add" and "pop" to "start" and "end" for clarity. Isolate CDI-specific data by adding Mode to PortHandler (based on what data string was requested and not yet terminated). Add formatted_ex for logging.
@Poikilos Poikilos force-pushed the delegate-state-to-link-and-flow-to-port branch from fbb7313 to f6c6361 Compare May 24, 2025 13:04
@bobjacobsen
Copy link
Owner

@Poikilos Thank you! Is this ready to merge now?

@Poikilos
Copy link
Contributor Author

Hi, I got back to this as you can see. I'm going to try to get CDI element names looking correct in the GUI shortly then I'll be ready.

…rocessor, make it a subclass, and separate format (XML) from space (CDI or FDI).
…and thereby implement accurate reporting regardless of using CLI or GUI). Improve some docstrings.
@Poikilos Poikilos force-pushed the delegate-state-to-link-and-flow-to-port branch from 2b735cf to dc7960e Compare January 12, 2026 23:32
…mit CID/RID/AMD while not Permitted (requires: Set state before sending AME, also fixed).
…f not Permitted, as per Standard; Test exact length of data when matching AME as per Standard. Move decodeControlFrameFormat to CanFrame as static method for modularity (no CanLink instance is required). Add noise rejection (move old handleData code to handleDataOptimized) (fix bobjacobsen#82 and fix bobjacobsen#83 before committing). Add reusable from*_hex_bytes functions. Make handleData and CanFrame constructor messages more explicit. Fix bobjacobsen#86: Add minimumState option to CanFrame so transition to Inhibited can send AMR after CIDs are queued but not sent, since enqueuing since enqueuing changes CanLink state immediately. Fix bobjacobsen#77: Always increment nextInternallyAssignedNodeID when used. Add explicit blockedReason function for tracing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants