feat: validate source address on ControllerApplication construction - #83
Open
RaulSMS wants to merge 1 commit into
Open
feat: validate source address on ControllerApplication construction#83RaulSMS wants to merge 1 commit into
RaulSMS wants to merge 1 commit into
Conversation
ControllerApplication previously accepted any device_address_preferred at construction time without checking it was a claimable source address (0..253), only validating later when an address change was requested via the J1939-81 claim procedure. An invalid address passed at construction would silently sit in an invalid state until the first claim attempt. Extract the existing 0..253 bounds check into a shared _check_source_address() static method, reused by both the constructor (raises ValueError for None-safe, out-of-range addresses) and _begin_address_claim() (preserves its existing warn-and-return-False behavior for the async claim path). Fixes #56 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
drewr95
approved these changes
Aug 28, 2026
khauersp
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ControllerApplication.__init__accepted anydevice_address_preferredwithout validating it was a claimable source address (0..253) — the only existing check lived inline in_begin_address_claim(the J1939-81 claim path), so an invalid address passed at construction would silently sit in an invalid state until the first claim attempt.ControllerApplication._check_source_address()static method.device_address_preferred(when notNone) and raisesValueErrorwith an explicit message stating the valid range, per the issue's acceptance criteria._begin_address_claim()now reuses the same shared check, preserving its existing behavior (log a warning and returnFalse) for the async runtime claim path — no behavior change there.Fixes #56
Test plan
test/test_ca.py: min valid (0), max valid (253),None(no preferred address), and invalid boundaries (-1,254NULL,255GLOBAL) all raise/pass as expected.pytest test/test_ca.py -q— 28 passedpytest . --pyargs -q— 533 passedruff check— cleanpyright— 0 errors