Skip to content

Only define owrxdone method with IRAM_ATTR in one place#26

Open
cleishm wants to merge 1 commit into
junkfix:mainfrom
cleishm:owrxdone
Open

Only define owrxdone method with IRAM_ATTR in one place#26
cleishm wants to merge 1 commit into
junkfix:mainfrom
cleishm:owrxdone

Conversation

@cleishm
Copy link
Copy Markdown

@cleishm cleishm commented Nov 5, 2025

Moves definition to .cpp, as it does not need to be accessible outside this library. This avoids warnings with declaring the attribute in multiple source files.

mathieucarbou added a commit to mathieucarbou/MycilaDS18 that referenced this pull request Nov 5, 2025
mathieucarbou added a commit to mathieucarbou/MycilaDS18 that referenced this pull request Nov 5, 2025
Comment thread OneWireESP32.cpp Outdated
#define OW_SLOT_RECOVERY 5
#define OW_TIMEOUT 50

IRAM_ATTR bool owrxdone(rmt_channel_handle_t ch, const rmt_rx_done_event_data_t *edata, void *udata);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can also be made static and constexpr

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point. If you want to push a change, please do! Otherwise I'll do it tomorrow.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good point. If you want to push a change, please do! Otherwise I'll do it tomorrow.

I have my own lib, depending on this one (https://github.com/mathieucarbou/MycilaDS18), which adds a more friendly user facing layer for arduino. So I use to watch for the good ideas to cherry pick ;-)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Pushed the change

Moves definition to .cpp, as it does not need to be accessible
outside this library.
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.

2 participants