Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add CAN transport layer #1488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AdamSlowik wants to merge 14 commits into mysensors:development
base: development
Choose a base branch
Loading
from AdamSlowik:development

Conversation

@AdamSlowik
Copy link

@AdamSlowik AdamSlowik commented May 3, 2021

Add new transport layer.
Uses github.com/coryjfowler/MCP_CAN_lib maintained by @coryjfowler.

henrikekblad, lachvoj, virtual-maker, and nschurando reacted with thumbs up emoji
Copy link
Author

Is there some simple way of running those jenkins tests on local machine? Perhaps with some docker assistance?

Copy link
Author

Details of build errors are no longer available. Can someone trigger jenkins job?

Copy link
Contributor

@AdamSlowik

Hi Adam, thank you for your extension of MySensors to include the CAN bus transport layer. I am glad that you are also so persistent in satisfying the toll gate.

The "Butler" step is passed now, perfect!
Unfortunately you are now at the same point as in PR #1522.

Strangely enough, the last two PR #1524 and PR #1525 have successfully passed the toll gate. I therefore wonder what is different about your PR.

My suspicions for possible causes are:

  1. your development branch is now 6 commits behind the development of MySensors repo. You could try to rebase your branch onto your upstream development.
  2. the successful PR's consisted of only one commit. So you could try to squash your commits and force push again.

See also my comments here:
1515#issuecomment-1030930615
1515#issuecomment-1030939724

mfalkvidd reacted with thumbs up emoji

Copy link
Contributor

@AdamSlowik Yes, after squash toll gate looks different.
I found 2 warnings in Jenkins from your new CAN code. I hope when you fix both, toll gate may pass.
Jenkins log

MySensors/hal/transport/CAN/MyTransportCAN.cpp:228:21: warning: unused variable 'to' [-Wunused-variable]
 long unsigned int to = (rxId & 0x0000FF00) >> 8;
 ^~
MySensors/examples/CANSwitch/CANSwitch.ino:96:17: warning: unused variable 'sentValue' [-Wunused-variable]
 static uint8_t sentValue=2;
 ^~~~~~~~~

Good luck 👍

AdamSlowik added a commit to AdamSlowik/MySensors that referenced this pull request Jun 29, 2022
Copy link
Author

According to:

https://ci.mysensors.org/blue/rest/organizations/jenkins/pipelines/MySensors/pipelines/MySensors/branches/PR-1488/runs/15/nodes/274/steps/290/log/?start=0

There are many warnings, but not related to My changes. Is there any other log file? Why other branches do not fail on those warnings?

Copy link
Author

@virtual-maker could You take a look at tool gate logs? Perhaps I missed something.

Copy link
Contributor

@AdamSlowik
Looks like this is the same problem with this new PJON driver stuff.
Toll gate (STM32F1 - Tests) — Warnings found
Details

In the full Jenkins log
I found this in lines around line# 8620:

In file included from /var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1488/workspace/MySensors/hal/transport/PJON/driver/PJON.h:62,
 from /var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1488/workspace/MySensors/MySensors.h:397,
 from /var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1488/workspace/MySensors/tests/Arduino/sketches/pjon_transport/pjon_transport.ino:24:
/var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1488/workspace/MySensors/hal/transport/PJON/driver/PJONDefines.h: In static member function 'static void PJONTools::parse_header(const uint8_t*, PJON_Packet_Info&)':
/var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1488/workspace/MySensors/hal/transport/PJON/driver/PJONDefines.h:415:31: warning: 'void* memset(void*, int, size_t)' clearing an object of non-trivial type 'struct PJON_Packet_Info'; use assignment or value-initialization instead [-Wclass-memaccess]
 415 | memset(&info, 0, sizeof info);
 | ^
/var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1488/workspace/MySensors/hal/transport/PJON/driver/PJONDefines.h:207:8: note: 'struct PJON_Packet_Info' declared here
 207 | struct PJON_Packet_Info {
 | ^~~~~~~~~~~~~~~~

There was a discussion in PR #1520 to exactly the same warnings in PJONDefines.h.
I assume that this memset(&info, 0, sizeof info); is not best practice and should be replaced.

virtual-maker pushed a commit to virtual-maker/MySensors that referenced this pull request Jul 29, 2022
Copy link

Great work! I'm interested in using can as a transport later as well. Leaving message to be notified of any progress. Let me know if I can contribute in some way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /