Skip to content

small refactor of some spi and timerwrapper functionality#631

Open
victor-Lopez25 wants to merge 14 commits intodevelopmentfrom
refactor/spi-timerwrapper
Open

small refactor of some spi and timerwrapper functionality#631
victor-Lopez25 wants to merge 14 commits intodevelopmentfrom
refactor/spi-timerwrapper

Conversation

@victor-Lopez25
Copy link
Copy Markdown
Contributor

timerwrapper:

  • Adds set_callback(void (*callback)(void*), void* callback_data) to set the callback and its data instead of needing to call configurexxbit() and set the period.
  • Adds set_limit_value(uint32_t arr) to set the arr, this will likely be changed to use a uint32_t type only when using a 32 bit timer, for now it is just an alias to instance->tim->ARR = arr;.

spi:

  • Adds transcieve with ptr + data explicitly instead of using a span since it's sometimes a pain in the ass to use.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 23, 2026

ST-LIB Release Plan

  • Current version: 5.2.0
  • Pending changesets: 5
  • Highest requested bump: minor
  • Next version if merged now: 5.3.0

Pending changes

  • minor add an ethernet connected check (.changesets/add_check_ethernet.md)
  • patch MPUDomain dynamic regions were configured using the linker symbol values instead of their addresses (that is the correct way of using linker symbols). That made non-cached regions cached, and caused harware undefined behaviour. (.changesets/hotfix-mpu-synamic-regions-linker-symbols.md)
  • patch fix pwm channel 4 config in TimerWrapper (.changesets/hotfix-pwm-channel4.md)
  • patch fix prescaler calculation in scheduler (.changesets/hotfix-sched-prescaler.md)
  • minor Small refactor of some spi and timerwrapper functionality (.changesets/refactor-spi-timerwrapper.md)

@victor-Lopez25 victor-Lopez25 marked this pull request as ready for review April 23, 2026 11:08
FoniksFox
FoniksFox previously approved these changes Apr 23, 2026
Copy link
Copy Markdown
Contributor

@FoniksFox FoniksFox left a comment

Choose a reason for hiding this comment

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

Will have to further improve these things, but it is good for now

@jorgesg82 jorgesg82 requested a review from Copilot April 23, 2026 16:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors timer and SPI wrapper APIs to make common configuration patterns less cumbersome, and tightens scheduler timer selection to 32-bit timers.

Changes:

  • Add TimerWrapper::set_callback(callback, callback_data) to set the ISR callback/data without configuring period.
  • Allow SCHEDULER_TIMER_DOMAIN to be set to TIM5 (32-bit) instead of TIM3.
  • Add a blocking SPI transceive(T* tx, T* rx, size_t count) overload (pointer + count).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
Inc/HALAL/Services/Time/TimerWrapper.hpp Adds set_callback helper that directly updates TimerDomain callback tables.
Inc/HALAL/Models/TimerDomain/TimerDomain.hpp Restricts scheduler timer selection to known 32-bit timers (2/5/23/24).
Inc/HALAL/Models/SPI/SPI2.hpp Adds pointer+count blocking transceive overload alongside existing span overloads.
.changesets/refactor-spi-timerwrapper.md Adds a changeset entry describing the refactor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Inc/HALAL/Services/Time/TimerWrapper.hpp
Comment thread Inc/HALAL/Services/Time/TimerWrapper.hpp
Comment thread Inc/HALAL/Services/Time/TimerWrapper.hpp
Comment thread Inc/HALAL/Models/SPI/SPI2.hpp
Comment thread .changesets/refactor-spi-timerwrapper.md Outdated
Comment thread .changesets/refactor-spi-timerwrapper.md Outdated
Comment thread Inc/HALAL/Models/SPI/SPI2.hpp
Comment thread Inc/HALAL/Models/SPI/SPI2.hpp
Copy link
Copy Markdown
Contributor

@FoniksFox FoniksFox left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants